From f9e13fabe7018535b3da5cd4f508ed2bd0d7f743 Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Mon, 2 Dec 2024 12:06:45 +0000 Subject: [PATCH 1/6] feat(framework) Refactor flwr ls table formatter --- src/py/flwr/cli/ls.py | 71 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 59 insertions(+), 12 deletions(-) diff --git a/src/py/flwr/cli/ls.py b/src/py/flwr/cli/ls.py index 54173f5f6018..4b0e3e109ade 100644 --- a/src/py/flwr/cli/ls.py +++ b/src/py/flwr/cli/ls.py @@ -139,13 +139,15 @@ def _init_channel(app: Path, federation_config: dict[str, Any]) -> grpc.Channel: return channel -def _format_run_table(run_dict: dict[int, Run], now_isoformat: str) -> Table: - """Format run status as a rich Table.""" +def _format_run(run_dict: dict[int, Run], now_isoformat: str) -> list[tuple[str, ...]]: + """Format run status to a list.""" table = Table(header_style="bold cyan", show_lines=True) def _format_datetime(dt: Optional[datetime]) -> str: return isoformat8601_utc(dt).replace("T", " ") if dt else "N/A" + run_status_list: list[tuple[str, ...]] = [] + # Add columns table.add_column( Text("Run ID", justify="center"), style="bright_white", overflow="fold" @@ -192,15 +194,60 @@ def _format_datetime(dt: Optional[datetime]) -> str: end_time = datetime.fromisoformat(now_isoformat) elapsed_time = end_time - running_at - table.add_row( - f"[bold]{run.run_id}[/bold]", - f"{run.fab_id} (v{run.fab_version})", - f"[{status_style}]{status_text}[/{status_style}]", - format_timedelta(elapsed_time), - _format_datetime(pending_at), - _format_datetime(running_at), - _format_datetime(finished_at), + run_status_list.append( + ( + f"{run.run_id}", + f"{run.fab_id}", + f"{run.fab_version}", + f"{run.fab_hash}", + f"[{status_style}]{status_text}[/{status_style}]", + format_timedelta(elapsed_time), + _format_datetime(pending_at), + _format_datetime(running_at), + _format_datetime(finished_at), + ) ) + return run_status_list + + +def _to_table(run_list: list[tuple[str, ...]]) -> Table: + """Format run status list to a rich Table.""" + table = Table(header_style="bold cyan", show_lines=True) + + # Add columns + table.add_column( + Text("Run ID", justify="center"), style="bright_white", overflow="fold" + ) + table.add_column(Text("FAB", justify="center"), style="dim white") + table.add_column(Text("Status", justify="center")) + table.add_column(Text("Elapsed", justify="center"), style="blue") + table.add_column(Text("Created At", justify="center"), style="dim white") + table.add_column(Text("Running At", justify="center"), style="dim white") + table.add_column(Text("Finished At", justify="center"), style="dim white") + + for row in run_list: + ( + run_id, + fab_id, + fab_version, + _, + status_text, + elapsed, + created_at, + running_at, + finished_at, + ) = row + formatted_row = ( + f"[bold]{run_id}[/bold]", + f"{fab_id} (v{fab_version})", + status_text, + elapsed, + created_at, + running_at, + finished_at, + ) + table.add_row(*formatted_row) + return table @@ -211,7 +258,7 @@ def _list_runs( res: ListRunsResponse = stub.ListRuns(ListRunsRequest()) run_dict = {run_id: run_from_proto(proto) for run_id, proto in res.run_dict.items()} - Console().print(_format_run_table(run_dict, res.now)) + Console().print(_to_table(_format_run(run_dict, res.now))) def _display_one_run( @@ -225,4 +272,4 @@ def _display_one_run( run_dict = {run_id: run_from_proto(proto) for run_id, proto in res.run_dict.items()} - Console().print(_format_run_table(run_dict, res.now)) + Console().print(_to_table(_format_run(run_dict, res.now))) From 6a96dfe2ce1ca06a26ac453fd567d75eedd345e8 Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Mon, 2 Dec 2024 12:18:16 +0000 Subject: [PATCH 2/6] Introduce try-except block to print JSON-formatted errors --- src/py/flwr/cli/ls.py | 94 +++++++++++++++++++++++-------------------- 1 file changed, 51 insertions(+), 43 deletions(-) diff --git a/src/py/flwr/cli/ls.py b/src/py/flwr/cli/ls.py index 4b0e3e109ade..51e3140c4963 100644 --- a/src/py/flwr/cli/ls.py +++ b/src/py/flwr/cli/ls.py @@ -70,52 +70,56 @@ def ls( ] = None, ) -> None: """List runs.""" - # Load and validate federation config - typer.secho("Loading project configuration... ", fg=typer.colors.BLUE) - - pyproject_path = app / FAB_CONFIG_FILE if app else None - config, errors, warnings = load_and_validate(path=pyproject_path) - config = validate_project_config(config, errors, warnings) - federation, federation_config = validate_federation_in_project_config( - federation, config - ) - - if "address" not in federation_config: - typer.secho( - "❌ `flwr ls` currently works with Exec API. Ensure that the correct" - "Exec API address is provided in the `pyproject.toml`.", - fg=typer.colors.RED, - bold=True, + try: + # Load and validate federation config + typer.secho("Loading project configuration... ", fg=typer.colors.BLUE) + + pyproject_path = app / FAB_CONFIG_FILE if app else None + config, errors, warnings = load_and_validate(path=pyproject_path) + config = validate_project_config(config, errors, warnings) + federation, federation_config = validate_federation_in_project_config( + federation, config ) - raise typer.Exit(code=1) - try: - if runs and run_id is not None: - raise ValueError( - "The options '--runs' and '--run-id' are mutually exclusive." + if "address" not in federation_config: + typer.secho( + "❌ `flwr ls` currently works with Exec API. Ensure that the correct" + "Exec API address is provided in the `pyproject.toml`.", + fg=typer.colors.RED, + bold=True, ) - - channel = _init_channel(app, federation_config) - stub = ExecStub(channel) - - # Display information about a specific run ID - if run_id is not None: - typer.echo(f"🔍 Displaying information for run ID {run_id}...") - _display_one_run(stub, run_id) - # By default, list all runs - else: - typer.echo("📄 Listing all runs...") - _list_runs(stub) - - except ValueError as err: - typer.secho( - f"❌ {err}", - fg=typer.colors.RED, - bold=True, - ) - raise typer.Exit(code=1) from err - finally: - channel.close() + raise typer.Exit(code=1) + + try: + if runs and run_id is not None: + raise ValueError( + "The options '--runs' and '--run-id' are mutually exclusive." + ) + + channel = _init_channel(app, federation_config) + stub = ExecStub(channel) + + # Display information about a specific run ID + if run_id is not None: + typer.echo(f"🔍 Displaying information for run ID {run_id}...") + _display_one_run(stub, run_id) + # By default, list all runs + else: + typer.echo("📄 Listing all runs...") + _list_runs(stub) + + except ValueError as err: + typer.secho( + f"❌ {err}", + fg=typer.colors.RED, + bold=True, + ) + raise typer.Exit(code=1) from err + finally: + channel.close() + # pylint: disable=broad-except, unused-variable + except (typer.Exit, SystemExit, Exception): + _print_json_error() def on_channel_state_change(channel_connectivity: str) -> None: @@ -273,3 +277,7 @@ def _display_one_run( run_dict = {run_id: run_from_proto(proto) for run_id, proto in res.run_dict.items()} Console().print(_to_table(_format_run(run_dict, res.now))) + + +def _print_json_error() -> None: + """Print error message as JSON.""" From 97b9a70f7a1867293e2b8478367d01f8e45c37ff Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Mon, 2 Dec 2024 12:28:20 +0000 Subject: [PATCH 3/6] Format --- src/py/flwr/cli/ls.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/py/flwr/cli/ls.py b/src/py/flwr/cli/ls.py index 51e3140c4963..7b726b6b26d4 100644 --- a/src/py/flwr/cli/ls.py +++ b/src/py/flwr/cli/ls.py @@ -255,9 +255,7 @@ def _to_table(run_list: list[tuple[str, ...]]) -> Table: return table -def _list_runs( - stub: ExecStub, -) -> None: +def _list_runs(stub: ExecStub) -> None: """List all runs.""" res: ListRunsResponse = stub.ListRuns(ListRunsRequest()) run_dict = {run_id: run_from_proto(proto) for run_id, proto in res.run_dict.items()} @@ -265,10 +263,7 @@ def _list_runs( Console().print(_to_table(_format_run(run_dict, res.now))) -def _display_one_run( - stub: ExecStub, - run_id: int, -) -> None: +def _display_one_run(stub: ExecStub, run_id: int) -> None: """Display information about a specific run.""" res: ListRunsResponse = stub.ListRuns(ListRunsRequest(run_id=run_id)) if not res.run_dict: From 392b9c337dc95dfdf0816f51b865936467c7707e Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Tue, 3 Dec 2024 11:02:33 +0000 Subject: [PATCH 4/6] Revert --- src/py/flwr/cli/ls.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/py/flwr/cli/ls.py b/src/py/flwr/cli/ls.py index 7b726b6b26d4..51e3140c4963 100644 --- a/src/py/flwr/cli/ls.py +++ b/src/py/flwr/cli/ls.py @@ -255,7 +255,9 @@ def _to_table(run_list: list[tuple[str, ...]]) -> Table: return table -def _list_runs(stub: ExecStub) -> None: +def _list_runs( + stub: ExecStub, +) -> None: """List all runs.""" res: ListRunsResponse = stub.ListRuns(ListRunsRequest()) run_dict = {run_id: run_from_proto(proto) for run_id, proto in res.run_dict.items()} @@ -263,7 +265,10 @@ def _list_runs(stub: ExecStub) -> None: Console().print(_to_table(_format_run(run_dict, res.now))) -def _display_one_run(stub: ExecStub, run_id: int) -> None: +def _display_one_run( + stub: ExecStub, + run_id: int, +) -> None: """Display information about a specific run.""" res: ListRunsResponse = stub.ListRuns(ListRunsRequest(run_id=run_id)) if not res.run_dict: From 3d22e45cdf3d4fde2c58d57660cd31bfa30cf301 Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Tue, 3 Dec 2024 13:30:29 +0000 Subject: [PATCH 5/6] Address comments --- src/py/flwr/cli/ls.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/py/flwr/cli/ls.py b/src/py/flwr/cli/ls.py index 4b0e3e109ade..f49dc3e25099 100644 --- a/src/py/flwr/cli/ls.py +++ b/src/py/flwr/cli/ls.py @@ -139,14 +139,14 @@ def _init_channel(app: Path, federation_config: dict[str, Any]) -> grpc.Channel: return channel -def _format_run(run_dict: dict[int, Run], now_isoformat: str) -> list[tuple[str, ...]]: - """Format run status to a list.""" +def _format_runs(run_dict: dict[int, Run], now_isoformat: str) -> list[tuple[str, ...]]: + """Format runs to a list.""" table = Table(header_style="bold cyan", show_lines=True) def _format_datetime(dt: Optional[datetime]) -> str: return isoformat8601_utc(dt).replace("T", " ") if dt else "N/A" - run_status_list: list[tuple[str, ...]] = [] + run_list: list[tuple[str, ...]] = [] # Add columns table.add_column( @@ -194,7 +194,7 @@ def _format_datetime(dt: Optional[datetime]) -> str: end_time = datetime.fromisoformat(now_isoformat) elapsed_time = end_time - running_at - run_status_list.append( + run_list.append( ( f"{run.run_id}", f"{run.fab_id}", @@ -207,11 +207,11 @@ def _format_datetime(dt: Optional[datetime]) -> str: _format_datetime(finished_at), ) ) - return run_status_list + return run_list def _to_table(run_list: list[tuple[str, ...]]) -> Table: - """Format run status list to a rich Table.""" + """Format the provided run list to a rich Table.""" table = Table(header_style="bold cyan", show_lines=True) # Add columns @@ -258,7 +258,7 @@ def _list_runs( res: ListRunsResponse = stub.ListRuns(ListRunsRequest()) run_dict = {run_id: run_from_proto(proto) for run_id, proto in res.run_dict.items()} - Console().print(_to_table(_format_run(run_dict, res.now))) + Console().print(_to_table(_format_runs(run_dict, res.now))) def _display_one_run( @@ -272,4 +272,4 @@ def _display_one_run( run_dict = {run_id: run_from_proto(proto) for run_id, proto in res.run_dict.items()} - Console().print(_to_table(_format_run(run_dict, res.now))) + Console().print(_to_table(_format_runs(run_dict, res.now))) From 066dfcf541fc2ef29918abac9f06b687f0bae790 Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Tue, 3 Dec 2024 13:52:44 +0000 Subject: [PATCH 6/6] Remove SystemExit --- src/py/flwr/cli/ls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/py/flwr/cli/ls.py b/src/py/flwr/cli/ls.py index 49247bc2a0d4..48e882bc178b 100644 --- a/src/py/flwr/cli/ls.py +++ b/src/py/flwr/cli/ls.py @@ -118,7 +118,7 @@ def ls( finally: channel.close() # pylint: disable=broad-except, unused-variable - except (typer.Exit, SystemExit, Exception): + except (typer.Exit, Exception): _print_json_error()