From f9e13fabe7018535b3da5cd4f508ed2bd0d7f743 Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Mon, 2 Dec 2024 12:06:45 +0000 Subject: [PATCH 01/18] 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 02/18] 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 bc761b5d5ba2a73336a84bc5cfdfd2755ee37137 Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Mon, 2 Dec 2024 12:25:28 +0000 Subject: [PATCH 03/18] Introduce JSON formatting function --- src/py/flwr/cli/ls.py | 63 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 54 insertions(+), 9 deletions(-) diff --git a/src/py/flwr/cli/ls.py b/src/py/flwr/cli/ls.py index 51e3140c4963..edd90b948f6f 100644 --- a/src/py/flwr/cli/ls.py +++ b/src/py/flwr/cli/ls.py @@ -15,6 +15,7 @@ """Flower command line interface `ls` command.""" +import json from datetime import datetime, timedelta from logging import DEBUG from pathlib import Path @@ -255,20 +256,61 @@ def _to_table(run_list: list[tuple[str, ...]]) -> Table: return table -def _list_runs( - stub: ExecStub, -) -> None: +def _to_json(run_list: list[tuple[str, ...]]) -> str: + """.""" + + def _remove_bbcode_tags(strings: tuple[str, ...]) -> tuple[str, ...]: + """Remove BBCode tags from the provided text.""" + # Regular expression pattern to match BBCode tags + bbcode_pattern = re.compile(r"\[/?\w+\]") + # Substitute BBCode tags with an empty string + return tuple(bbcode_pattern.sub("", s) for s in strings) + + # runs_dict: Dict[str, list[Dict[str, str]]] = {"runs": []} + runs_list = [] + for row in run_list: + row = _remove_bbcode_tags(row) + ( + run_id, + fab_id, + fab_version, + fab_hash, + status_text, + elapsed, + created_at, + running_at, + finished_at, + ) = row + runs_list.append( + { + "run-id": run_id, + "fab-id": fab_id, + "fab-name": fab_id.split("/")[-1], + "fab-version": fab_version, + "fab-hash": fab_hash[:8], + "status": status_text, + "elapsed": elapsed, + "created-at": created_at, + "running-at": running_at, + "finished-at": finished_at, + } + ) + + return json.dumps({"runs": runs_list}) + + +def _list_runs(stub: ExecStub, ls_format: str = "table") -> 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()} - Console().print(_to_table(_format_run(run_dict, res.now))) + if ls_format == "table": + Console().print(_to_table(_format_run(run_dict, res.now))) + else: + Console().print_json(_to_json(_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, ls_format: str = "table") -> None: """Display information about a specific run.""" res: ListRunsResponse = stub.ListRuns(ListRunsRequest(run_id=run_id)) if not res.run_dict: @@ -276,7 +318,10 @@ 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))) + if ls_format == "table": + Console().print(_to_table(_format_run(run_dict, res.now))) + else: + Console().print_json(_to_json(_format_run(run_dict, res.now))) def _print_json_error() -> None: From 97b9a70f7a1867293e2b8478367d01f8e45c37ff Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Mon, 2 Dec 2024 12:28:20 +0000 Subject: [PATCH 04/18] 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 5de9505734a3258b436312bcc38c9f9c82cdec82 Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Mon, 2 Dec 2024 12:30:30 +0000 Subject: [PATCH 05/18] Add missing import --- src/py/flwr/cli/ls.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/py/flwr/cli/ls.py b/src/py/flwr/cli/ls.py index edd90b948f6f..1709fe94b8c3 100644 --- a/src/py/flwr/cli/ls.py +++ b/src/py/flwr/cli/ls.py @@ -16,6 +16,7 @@ import json +import re from datetime import datetime, timedelta from logging import DEBUG from pathlib import Path From 28cf26c1c42564c62d8ee07af482169539b7f493 Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Mon, 2 Dec 2024 12:34:01 +0000 Subject: [PATCH 06/18] Update docstring --- 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 1709fe94b8c3..aa3a519b11f3 100644 --- a/src/py/flwr/cli/ls.py +++ b/src/py/flwr/cli/ls.py @@ -258,7 +258,7 @@ def _to_table(run_list: list[tuple[str, ...]]) -> Table: def _to_json(run_list: list[tuple[str, ...]]) -> str: - """.""" + """Format run status list to a JSON formatted string.""" def _remove_bbcode_tags(strings: tuple[str, ...]) -> tuple[str, ...]: """Remove BBCode tags from the provided text.""" From bd1c86a24c4c8dfa1b956d34a3fcac6bb1c5cd50 Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Tue, 3 Dec 2024 10:33:42 +0000 Subject: [PATCH 07/18] Add output formats to constants --- src/py/flwr/cli/ls.py | 21 +++++++++++---------- src/py/flwr/common/constant.py | 9 +++++++++ 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/py/flwr/cli/ls.py b/src/py/flwr/cli/ls.py index aa3a519b11f3..1f490fd28449 100644 --- a/src/py/flwr/cli/ls.py +++ b/src/py/flwr/cli/ls.py @@ -34,7 +34,7 @@ validate_federation_in_project_config, validate_project_config, ) -from flwr.common.constant import FAB_CONFIG_FILE, SubStatus +from flwr.common.constant import FAB_CONFIG_FILE, CliOutputFormat, SubStatus from flwr.common.date import format_timedelta, isoformat8601_utc from flwr.common.grpc import GRPC_MAX_MESSAGE_LENGTH, create_channel from flwr.common.logger import log @@ -267,7 +267,6 @@ def _remove_bbcode_tags(strings: tuple[str, ...]) -> tuple[str, ...]: # Substitute BBCode tags with an empty string return tuple(bbcode_pattern.sub("", s) for s in strings) - # runs_dict: Dict[str, list[Dict[str, str]]] = {"runs": []} runs_list = [] for row in run_list: row = _remove_bbcode_tags(row) @@ -300,18 +299,20 @@ def _remove_bbcode_tags(strings: tuple[str, ...]) -> tuple[str, ...]: return json.dumps({"runs": runs_list}) -def _list_runs(stub: ExecStub, ls_format: str = "table") -> None: +def _list_runs(stub: ExecStub, output_format: str = CliOutputFormat.default) -> 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()} - if ls_format == "table": - Console().print(_to_table(_format_run(run_dict, res.now))) - else: + if output_format == CliOutputFormat.json: Console().print_json(_to_json(_format_run(run_dict, res.now))) + else: + Console().print(_to_table(_format_run(run_dict, res.now))) -def _display_one_run(stub: ExecStub, run_id: int, ls_format: str = "table") -> None: +def _display_one_run( + stub: ExecStub, run_id: int, output_format: str = CliOutputFormat.default +) -> None: """Display information about a specific run.""" res: ListRunsResponse = stub.ListRuns(ListRunsRequest(run_id=run_id)) if not res.run_dict: @@ -319,10 +320,10 @@ def _display_one_run(stub: ExecStub, run_id: int, ls_format: str = "table") -> N run_dict = {run_id: run_from_proto(proto) for run_id, proto in res.run_dict.items()} - if ls_format == "table": - Console().print(_to_table(_format_run(run_dict, res.now))) - else: + if output_format == CliOutputFormat.json: Console().print_json(_to_json(_format_run(run_dict, res.now))) + else: + Console().print(_to_table(_format_run(run_dict, res.now))) def _print_json_error() -> None: diff --git a/src/py/flwr/common/constant.py b/src/py/flwr/common/constant.py index ec84aa984fde..ba72964449d8 100644 --- a/src/py/flwr/common/constant.py +++ b/src/py/flwr/common/constant.py @@ -17,6 +17,8 @@ from __future__ import annotations +from enum import Enum + MISSING_EXTRA_REST = """ Extra dependencies required for using the REST-based Fleet API are missing. @@ -181,3 +183,10 @@ class SubStatus: def __new__(cls) -> SubStatus: """Prevent instantiation.""" raise TypeError(f"{cls.__name__} cannot be instantiated.") + + +class CliOutputFormat(str, Enum): + """Define output format for `flwr` CLI commands.""" + + default = "default" # pylint: disable=invalid-name + json = "json" # pylint: disable=invalid-name From 392b9c337dc95dfdf0816f51b865936467c7707e Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Tue, 3 Dec 2024 11:02:33 +0000 Subject: [PATCH 08/18] 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 605d5768cf74e4d26edb2508aa48317584aff1d2 Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Tue, 3 Dec 2024 11:03:33 +0000 Subject: [PATCH 09/18] Break arg lines --- 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 1f490fd28449..3578045c1ffa 100644 --- a/src/py/flwr/cli/ls.py +++ b/src/py/flwr/cli/ls.py @@ -299,7 +299,10 @@ def _remove_bbcode_tags(strings: tuple[str, ...]) -> tuple[str, ...]: return json.dumps({"runs": runs_list}) -def _list_runs(stub: ExecStub, output_format: str = CliOutputFormat.default) -> None: +def _list_runs( + stub: ExecStub, + output_format: str = CliOutputFormat.default, +) -> 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()} @@ -311,7 +314,9 @@ def _list_runs(stub: ExecStub, output_format: str = CliOutputFormat.default) -> def _display_one_run( - stub: ExecStub, run_id: int, output_format: str = CliOutputFormat.default + stub: ExecStub, + run_id: int, + output_format: str = CliOutputFormat.default, ) -> None: """Display information about a specific run.""" res: ListRunsResponse = stub.ListRuns(ListRunsRequest(run_id=run_id)) From 3d22e45cdf3d4fde2c58d57660cd31bfa30cf301 Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Tue, 3 Dec 2024 13:30:29 +0000 Subject: [PATCH 10/18] 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 11/18] 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() From 69336adc6f918aa48f94358041aeb059f1cc5742 Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Tue, 3 Dec 2024 14:40:51 +0000 Subject: [PATCH 12/18] Address comment --- src/py/flwr/cli/ls.py | 8 ++++---- src/py/flwr/common/constant.py | 8 ++++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/py/flwr/cli/ls.py b/src/py/flwr/cli/ls.py index 03bcccd71d3f..ef61ed12624d 100644 --- a/src/py/flwr/cli/ls.py +++ b/src/py/flwr/cli/ls.py @@ -301,13 +301,13 @@ def _remove_bbcode_tags(strings: tuple[str, ...]) -> tuple[str, ...]: def _list_runs( stub: ExecStub, - output_format: str = CliOutputFormat.default, + output_format: str = CliOutputFormat.DEFAULT, ) -> 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()} - if output_format == CliOutputFormat.json: + if output_format == CliOutputFormat.JSON: Console().print_json(_to_json(_format_runs(run_dict, res.now))) else: Console().print(_to_table(_format_runs(run_dict, res.now))) @@ -316,7 +316,7 @@ def _list_runs( def _display_one_run( stub: ExecStub, run_id: int, - output_format: str = CliOutputFormat.default, + output_format: str = CliOutputFormat.DEFAULT, ) -> None: """Display information about a specific run.""" res: ListRunsResponse = stub.ListRuns(ListRunsRequest(run_id=run_id)) @@ -325,7 +325,7 @@ def _display_one_run( run_dict = {run_id: run_from_proto(proto) for run_id, proto in res.run_dict.items()} - if output_format == CliOutputFormat.json: + if output_format == CliOutputFormat.JSON: Console().print_json(_to_json(_format_runs(run_dict, res.now))) else: Console().print(_to_table(_format_runs(run_dict, res.now))) diff --git a/src/py/flwr/common/constant.py b/src/py/flwr/common/constant.py index ba72964449d8..f4c60b7ecbb3 100644 --- a/src/py/flwr/common/constant.py +++ b/src/py/flwr/common/constant.py @@ -188,5 +188,9 @@ def __new__(cls) -> SubStatus: class CliOutputFormat(str, Enum): """Define output format for `flwr` CLI commands.""" - default = "default" # pylint: disable=invalid-name - json = "json" # pylint: disable=invalid-name + DEFAULT = "default" + JSON = "json" + + def __new__(cls) -> CliOutputFormat: + """Prevent instantiation.""" + raise TypeError(f"{cls.__name__} cannot be instantiated.") From ab8e4d53850e0e51cd0994e9768241a50b85d92b Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Tue, 3 Dec 2024 15:53:14 +0000 Subject: [PATCH 13/18] Fix --- src/py/flwr/common/constant.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/py/flwr/common/constant.py b/src/py/flwr/common/constant.py index f4c60b7ecbb3..ce5840ddddfb 100644 --- a/src/py/flwr/common/constant.py +++ b/src/py/flwr/common/constant.py @@ -17,8 +17,6 @@ from __future__ import annotations -from enum import Enum - MISSING_EXTRA_REST = """ Extra dependencies required for using the REST-based Fleet API are missing. @@ -185,7 +183,7 @@ def __new__(cls) -> SubStatus: raise TypeError(f"{cls.__name__} cannot be instantiated.") -class CliOutputFormat(str, Enum): +class CliOutputFormat: """Define output format for `flwr` CLI commands.""" DEFAULT = "default" From 3c1808c40dfca6ea5213e5993e2abdead1cc8398 Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Wed, 4 Dec 2024 08:06:45 +0000 Subject: [PATCH 14/18] Move fn to utils --- src/py/flwr/cli/ls.py | 11 ++--------- src/py/flwr/cli/utils.py | 8 ++++++++ 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/py/flwr/cli/ls.py b/src/py/flwr/cli/ls.py index ef61ed12624d..42d657a4ca48 100644 --- a/src/py/flwr/cli/ls.py +++ b/src/py/flwr/cli/ls.py @@ -16,7 +16,6 @@ import json -import re from datetime import datetime, timedelta from logging import DEBUG from pathlib import Path @@ -34,6 +33,7 @@ validate_federation_in_project_config, validate_project_config, ) +from flwr.cli.utils import remove_bbcode_tags from flwr.common.constant import FAB_CONFIG_FILE, CliOutputFormat, SubStatus from flwr.common.date import format_timedelta, isoformat8601_utc from flwr.common.grpc import GRPC_MAX_MESSAGE_LENGTH, create_channel @@ -260,16 +260,9 @@ def _to_table(run_list: list[tuple[str, ...]]) -> Table: def _to_json(run_list: list[tuple[str, ...]]) -> str: """Format run status list to a JSON formatted string.""" - def _remove_bbcode_tags(strings: tuple[str, ...]) -> tuple[str, ...]: - """Remove BBCode tags from the provided text.""" - # Regular expression pattern to match BBCode tags - bbcode_pattern = re.compile(r"\[/?\w+\]") - # Substitute BBCode tags with an empty string - return tuple(bbcode_pattern.sub("", s) for s in strings) - runs_list = [] for row in run_list: - row = _remove_bbcode_tags(row) + row = remove_bbcode_tags(row) ( run_id, fab_id, diff --git a/src/py/flwr/cli/utils.py b/src/py/flwr/cli/utils.py index e725fdd3f951..2ac2e8782068 100644 --- a/src/py/flwr/cli/utils.py +++ b/src/py/flwr/cli/utils.py @@ -136,3 +136,11 @@ def get_sha256_hash(file_path: Path) -> str: break sha256.update(data) return sha256.hexdigest() + + +def remove_bbcode_tags(strings: tuple[str, ...]) -> tuple[str, ...]: + """Remove BBCode tags from the provided text.""" + # Regular expression pattern to match BBCode tags + bbcode_pattern = re.compile(r"\[/?\w+\]") + # Substitute BBCode tags with an empty string + return tuple(bbcode_pattern.sub("", s) for s in strings) From 51c5c1cffdc32bd75082fa392ef51b573af64769 Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Wed, 4 Dec 2024 11:30:03 +0000 Subject: [PATCH 15/18] Lint --- src/py/flwr/cli/ls.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/py/flwr/cli/ls.py b/src/py/flwr/cli/ls.py index 2fb986e618a8..60cb092a5487 100644 --- a/src/py/flwr/cli/ls.py +++ b/src/py/flwr/cli/ls.py @@ -259,7 +259,6 @@ def _to_table(run_list: list[tuple[str, ...]]) -> Table: def _to_json(run_list: list[tuple[str, ...]]) -> str: """Format run status list to a JSON formatted string.""" - runs_list = [] for row in run_list: row = remove_bbcode_tags(row) From 52838e59e9f3a64043f72121d5a962b7357a7bff Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Wed, 4 Dec 2024 11:52:55 +0000 Subject: [PATCH 16/18] Remove bbcode function, move format --- src/py/flwr/cli/ls.py | 36 +++++++++++------------------------- src/py/flwr/cli/utils.py | 8 -------- 2 files changed, 11 insertions(+), 33 deletions(-) diff --git a/src/py/flwr/cli/ls.py b/src/py/flwr/cli/ls.py index 60cb092a5487..5b20da186826 100644 --- a/src/py/flwr/cli/ls.py +++ b/src/py/flwr/cli/ls.py @@ -33,7 +33,6 @@ validate_federation_in_project_config, validate_project_config, ) -from flwr.cli.utils import remove_bbcode_tags from flwr.common.constant import FAB_CONFIG_FILE, CliOutputFormat, SubStatus from flwr.common.date import format_timedelta, isoformat8601_utc from flwr.common.grpc import GRPC_MAX_MESSAGE_LENGTH, create_channel @@ -147,24 +146,12 @@ def _init_channel(app: Path, federation_config: dict[str, Any]) -> grpc.Channel: 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_list: list[tuple[str, ...]] = [] - # 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") - # Add rows for run in sorted( run_dict.values(), key=lambda x: datetime.fromisoformat(x.pending_at) @@ -175,15 +162,6 @@ def _format_datetime(dt: Optional[datetime]) -> str: else: status_text = f"{run.status.status}:{run.status.sub_status}" - # Style the status based on its value - sub_status = run.status.sub_status - if sub_status == SubStatus.COMPLETED: - status_style = "green" - elif sub_status == SubStatus.FAILED: - status_style = "red" - else: - status_style = "yellow" - # Convert isoformat to datetime pending_at = datetime.fromisoformat(run.pending_at) if run.pending_at else None running_at = datetime.fromisoformat(run.running_at) if run.running_at else None @@ -206,7 +184,7 @@ def _format_datetime(dt: Optional[datetime]) -> str: f"{run.fab_id}", f"{run.fab_version}", f"{run.fab_hash}", - f"[{status_style}]{status_text}[/{status_style}]", + f"{status_text}", format_timedelta(elapsed_time), _format_datetime(pending_at), _format_datetime(running_at), @@ -243,10 +221,19 @@ def _to_table(run_list: list[tuple[str, ...]]) -> Table: running_at, finished_at, ) = row + # Style the status based on its value + sub_status = status_text.rsplit(":", maxsplit=1)[-1] + if sub_status == SubStatus.COMPLETED: + status_style = "green" + elif sub_status == SubStatus.FAILED: + status_style = "red" + else: + status_style = "yellow" + formatted_row = ( f"[bold]{run_id}[/bold]", f"{fab_id} (v{fab_version})", - status_text, + f"[{status_style}]{status_text}[/{status_style}]", elapsed, created_at, running_at, @@ -261,7 +248,6 @@ def _to_json(run_list: list[tuple[str, ...]]) -> str: """Format run status list to a JSON formatted string.""" runs_list = [] for row in run_list: - row = remove_bbcode_tags(row) ( run_id, fab_id, diff --git a/src/py/flwr/cli/utils.py b/src/py/flwr/cli/utils.py index 2ac2e8782068..e725fdd3f951 100644 --- a/src/py/flwr/cli/utils.py +++ b/src/py/flwr/cli/utils.py @@ -136,11 +136,3 @@ def get_sha256_hash(file_path: Path) -> str: break sha256.update(data) return sha256.hexdigest() - - -def remove_bbcode_tags(strings: tuple[str, ...]) -> tuple[str, ...]: - """Remove BBCode tags from the provided text.""" - # Regular expression pattern to match BBCode tags - bbcode_pattern = re.compile(r"\[/?\w+\]") - # Substitute BBCode tags with an empty string - return tuple(bbcode_pattern.sub("", s) for s in strings) From 09d1647c94ffb551ae3fe83d2de43981ae77a962 Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Wed, 4 Dec 2024 12:16:02 +0000 Subject: [PATCH 17/18] Update types --- src/py/flwr/cli/ls.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/py/flwr/cli/ls.py b/src/py/flwr/cli/ls.py index 5b20da186826..d0271f417fbb 100644 --- a/src/py/flwr/cli/ls.py +++ b/src/py/flwr/cli/ls.py @@ -45,6 +45,8 @@ ) from flwr.proto.exec_pb2_grpc import ExecStub +_RunListType = tuple[int, str, str, str, str, str, str, str, str] + def ls( app: Annotated[ @@ -144,13 +146,13 @@ def _init_channel(app: Path, federation_config: dict[str, Any]) -> grpc.Channel: return channel -def _format_runs(run_dict: dict[int, Run], now_isoformat: str) -> list[tuple[str, ...]]: +def _format_runs(run_dict: dict[int, Run], now_isoformat: str) -> list[_RunListType]: """Format runs to a list.""" def _format_datetime(dt: Optional[datetime]) -> str: return isoformat8601_utc(dt).replace("T", " ") if dt else "N/A" - run_list: list[tuple[str, ...]] = [] + run_list: list[_RunListType] = [] # Add rows for run in sorted( @@ -180,11 +182,11 @@ def _format_datetime(dt: Optional[datetime]) -> str: run_list.append( ( - f"{run.run_id}", - f"{run.fab_id}", - f"{run.fab_version}", - f"{run.fab_hash}", - f"{status_text}", + run.run_id, + run.fab_id, + run.fab_version, + run.fab_hash, + status_text, format_timedelta(elapsed_time), _format_datetime(pending_at), _format_datetime(running_at), @@ -194,7 +196,7 @@ def _format_datetime(dt: Optional[datetime]) -> str: return run_list -def _to_table(run_list: list[tuple[str, ...]]) -> Table: +def _to_table(run_list: list[_RunListType]) -> Table: """Format the provided run list to a rich Table.""" table = Table(header_style="bold cyan", show_lines=True) @@ -244,7 +246,7 @@ def _to_table(run_list: list[tuple[str, ...]]) -> Table: return table -def _to_json(run_list: list[tuple[str, ...]]) -> str: +def _to_json(run_list: list[_RunListType]) -> str: """Format run status list to a JSON formatted string.""" runs_list = [] for row in run_list: From b5952ca103823246c409a118a3e1b1b8bd80a672 Mon Sep 17 00:00:00 2001 From: Chong Shen Ng Date: Wed, 4 Dec 2024 12:26:29 +0000 Subject: [PATCH 18/18] Refactor --- src/py/flwr/cli/ls.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/py/flwr/cli/ls.py b/src/py/flwr/cli/ls.py index d0271f417fbb..622dc9f095ed 100644 --- a/src/py/flwr/cli/ls.py +++ b/src/py/flwr/cli/ls.py @@ -287,10 +287,11 @@ 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()} + formatted_runs = _format_runs(run_dict, res.now) if output_format == CliOutputFormat.JSON: - Console().print_json(_to_json(_format_runs(run_dict, res.now))) + Console().print_json(_to_json(formatted_runs)) else: - Console().print(_to_table(_format_runs(run_dict, res.now))) + Console().print(_to_table(formatted_runs)) def _display_one_run( @@ -305,10 +306,11 @@ def _display_one_run( run_dict = {run_id: run_from_proto(proto) for run_id, proto in res.run_dict.items()} + formatted_runs = _format_runs(run_dict, res.now) if output_format == CliOutputFormat.JSON: - Console().print_json(_to_json(_format_runs(run_dict, res.now))) + Console().print_json(_to_json(formatted_runs)) else: - Console().print(_to_table(_format_runs(run_dict, res.now))) + Console().print(_to_table(formatted_runs)) def _print_json_error() -> None: