Skip to content

fix(framework): Fix flwr no args update check #6939

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
mohammadnaseri wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

fix(framework): Fix flwr no args update check #6939

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0c2ef99
Edit
mohammadnaseri Apr 2, 2026
bc14a54
Edit
mohammadnaseri Apr 2, 2026
5990d1c
Merge branch 'main' into fix-flwr-no-args-update-check
danieljanes Apr 2, 2026
4707f96
Edit
mohammadnaseri Apr 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion framework/py/flwr/cli/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ class CommandKwargs(TypedDict):
fg=typer.colors.BRIGHT_YELLOW,
bold=True,
),
no_args_is_help=True,
context_settings={"help_option_names": ["-h", "--help"]},
)

Expand Down Expand Up @@ -151,6 +150,7 @@ def _is_json_output_requested(argv: list[str]) -> bool:

@app.callback(invoke_without_command=True)
def main(
ctx: typer.Context,
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep callback callable without injected Typer context

Adding a required ctx parameter breaks direct invocations of main outside Typer’s command runner, which now raises TypeError before the callback logic runs. In this repo, framework/py/flwr/cli/cli_test.py calls app_module.main(version=False) directly to verify update-check behavior, so this change introduces an immediate regression for those call paths unless ctx is made optional or those callers are updated.

Useful? React with 👍 / 👎.

version: bool = typer.Option(
None,
"-V",
Expand All @@ -167,6 +167,10 @@ def main(
typer.secho(f"Flower version: {package_version}", fg="blue")
raise typer.Exit()

if ctx.invoked_subcommand is None and len(sys.argv) == 1:
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The no-args detection uses len(sys.argv) == 1, which couples behavior to global process state and can be incorrect for programmatic invocations/tests where sys.argv doesn’t reflect the arguments passed to Typer. Prefer deriving this from the Click/Typer context (e.g., checking whether any args/subcommand were provided via the context) instead of sys.argv.

Suggested change
if ctx.invoked_subcommand is None and len(sys.argv) == 1:
if ctx.invoked_subcommand is None and not ctx.args and not ctx.params:

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Detect no-arg invocations from Click context

Checking len(sys.argv) == 1 is not a reliable way to detect an empty flwr invocation when the CLI is called programmatically with explicit args (for example via Click/Typer test runners), because Click parses the provided arg list without updating process-wide sys.argv. In those calls, ctx.invoked_subcommand is None but this condition is false, so the callback skips the help/exit path and returns success with no output, which is a behavioral regression from the prior no_args_is_help=True behavior.

Useful? React with 👍 / 👎.

typer.echo(ctx.get_help())
raise typer.Exit(code=2)
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new no-args handling prints help and then exits with code=2. Previously no_args_is_help=True would show help without treating it as a usage error; changing the exit code can be a breaking CLI behavior change for scripts. If the intent is still “show help on bare flwr”, consider exiting with code 0 (or otherwise matching the previous Click/Typer behavior).

Suggested change
raise typer.Exit(code=2)
raise typer.Exit()

Copilot uses AI. Check for mistakes.


if __name__ == "__main__":
app()
13 changes: 9 additions & 4 deletions framework/py/flwr/cli/cli_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,22 @@
from typing import Any
from unittest.mock import patch

import click
import pytest
from typer.testing import CliRunner

from flwr.supercore.version import package_version

from . import app as app_module
from .app import app
from .app import app, typer_click_object

runner = CliRunner()


def _make_context() -> click.Context:
return click.Context(typer_click_object)


def _invoke_flwr(args: list[str]) -> Any:
with patch("flwr.cli.app.warn_if_flwr_update_available"):
return runner.invoke(app, args)
Expand Down Expand Up @@ -135,7 +140,7 @@ def _raise_sentinel(process_name: str | None = None) -> None:
monkeypatch.setattr(app_module, "warn_if_flwr_update_available", _raise_sentinel)

with pytest.raises(_SentinelError):
app_module.main(version=False)
app_module.main(_make_context(), version=False)

assert captured == {"process_name": "flwr"}

Expand All @@ -156,7 +161,7 @@ def _warn(*_args: Any, **_kwargs: Any) -> None:

monkeypatch.setattr(app_module, "warn_if_flwr_update_available", _warn)

app_module.main(version=False)
app_module.main(_make_context(), version=False)

assert called is False

Expand All @@ -181,7 +186,7 @@ def _warn(*_args: Any, **_kwargs: Any) -> None:

monkeypatch.setattr(app_module, "warn_if_flwr_update_available", _warn)

app_module.main(version=False)
app_module.main(_make_context(), version=False)

assert called is should_call

Expand Down
Loading