Skip to content

Commit 934c3a2

Browse files
yan-gao-GYpanh99
andauthored
feat(framework): Update control servicer for flwr run with app ID (#6085)
Co-authored-by: Heng Pan <pan@flower.ai>
1 parent 7ce7071 commit 934c3a2

File tree

11 files changed

+448
-191
lines changed

11 files changed

+448
-191
lines changed

framework/py/flwr/cli/app_cmd/review.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
load_private_key,
3737
sign_message,
3838
)
39+
from flwr.supercore.utils import parse_app_spec, request_download_link
3940

4041
from ..auth_plugin.oidc_cli_plugin import OidcCliPlugin
4142
from ..config_utils import (
@@ -45,7 +46,7 @@
4546
)
4647
from ..constant import FEDERATION_CONFIG_HELP_MESSAGE
4748
from ..install import install_from_fab
48-
from ..utils import load_cli_auth_plugin, parse_app_spec, request_download_link
49+
from ..utils import load_cli_auth_plugin
4950

5051
TRY_AGAIN_MESSAGE = "Please try again or press CTRL+C to abort.\n"
5152

@@ -104,12 +105,21 @@ def review(
104105
token = auth_plugin.access_token
105106

106107
# Validate app version and ID format
107-
app_id, app_version = parse_app_spec(app_spec)
108+
try:
109+
app_id, app_version = parse_app_spec(app_spec)
110+
except ValueError as e:
111+
typer.secho(f"❌ {e}", fg=typer.colors.RED, err=True)
112+
raise typer.Exit(code=1) from e
108113

109114
# Download FAB
110115
typer.secho("Downloading FAB... ", fg=typer.colors.BLUE)
111116
url = f"{PLATFORM_API_URL}/hub/fetch-fab"
112-
presigned_url = request_download_link(app_id, app_version, url, "fab_url")
117+
try:
118+
presigned_url, _ = request_download_link(app_id, app_version, url, "fab_url")
119+
except ValueError as e:
120+
typer.secho(f"❌ {e}", fg=typer.colors.RED, err=True)
121+
raise typer.Exit(code=1) from e
122+
113123
fab_bytes = _download_fab(presigned_url)
114124

115125
# Unpack FAB

framework/py/flwr/cli/new/new.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,12 @@
2727
import typer
2828

2929
from flwr.supercore.constant import PLATFORM_API_URL
30+
from flwr.supercore.utils import parse_app_spec, request_download_link
3031

3132
from ..utils import (
3233
is_valid_project_name,
33-
parse_app_spec,
3434
prompt_options,
3535
prompt_text,
36-
request_download_link,
3736
sanitize_project_name,
3837
)
3938

@@ -205,7 +204,12 @@ def _download_zip_to_memory(presigned_url: str) -> io.BytesIO:
205204
def download_remote_app_via_api(app_spec: str) -> None:
206205
"""Download App from Platform API."""
207206
# Validate app version and ID format
208-
app_id, app_version = parse_app_spec(app_spec)
207+
try:
208+
app_id, app_version = parse_app_spec(app_spec)
209+
except ValueError as e:
210+
typer.secho(f"❌ {e}", fg=typer.colors.RED, err=True)
211+
raise typer.Exit(code=1) from e
212+
209213
app_name = app_id.split("/")[1]
210214

211215
project_dir = Path.cwd() / app_name
@@ -228,7 +232,11 @@ def download_remote_app_via_api(app_spec: str) -> None:
228232
)
229233
# Fetch ZIP downloading URL
230234
url = f"{PLATFORM_API_URL}/hub/fetch-zip"
231-
presigned_url = request_download_link(app_id, app_version, url, "zip_url")
235+
try:
236+
presigned_url, _ = request_download_link(app_id, app_version, url, "zip_url")
237+
except ValueError as e:
238+
typer.secho(f"❌ {e}", fg=typer.colors.RED, err=True)
239+
raise typer.Exit(code=1) from e
232240

233241
print(
234242
typer.style(

framework/py/flwr/cli/run/run.py

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -46,14 +46,10 @@
4646
from flwr.proto.control_pb2 import StartRunRequest # pylint: disable=E0611
4747
from flwr.proto.control_pb2_grpc import ControlStub
4848
from flwr.supercore.constant import NOOP_FEDERATION
49+
from flwr.supercore.utils import parse_app_spec
4950

5051
from ..log import start_stream
51-
from ..utils import (
52-
flwr_cli_grpc_exc_handler,
53-
init_channel,
54-
load_cli_auth_plugin,
55-
parse_app_spec,
56-
)
52+
from ..utils import flwr_cli_grpc_exc_handler, init_channel, load_cli_auth_plugin
5753

5854
CONN_REFRESH_PERIOD = 60 # Connection refresh period for log streaming (seconds)
5955

@@ -111,8 +107,15 @@ def run(
111107
app_spec = None
112108
if (app_str := str(app)).startswith("@"):
113109
# Validate app version and ID format
114-
_ = parse_app_spec(app_str)
110+
try:
111+
_ = parse_app_spec(app_str)
112+
except ValueError as e:
113+
typer.secho(f"❌ {e}", fg=typer.colors.RED, err=True)
114+
raise typer.Exit(code=1) from e
115+
115116
app_spec = app_str
117+
# Set `app` to current directory for credential storage
118+
app = Path(".")
116119
is_remote_app = app_spec is not None
117120

118121
typer.secho("Loading project configuration... ", fg=typer.colors.BLUE)
@@ -212,15 +215,7 @@ def _run_with_control_api(
212215
f"🎊 Successfully started run {res.run_id}", fg=typer.colors.GREEN
213216
)
214217
else:
215-
if is_remote_app:
216-
typer.secho(
217-
"❌ Failed to start run. Please check that the provided "
218-
"app identifier (@account_name/app_name) is correct.",
219-
fg=typer.colors.RED,
220-
err=True,
221-
)
222-
else:
223-
typer.secho("❌ Failed to start run", fg=typer.colors.RED, err=True)
218+
typer.secho("❌ Failed to start run", fg=typer.colors.RED, err=True)
224219
raise typer.Exit(code=1)
225220

226221
if output_format == CliOutputFormat.JSON:

framework/py/flwr/cli/utils.py

Lines changed: 0 additions & 137 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525

2626
import grpc
2727
import pathspec
28-
import requests
2928
import typer
3029

3130
from flwr.common.constant import (
@@ -48,8 +47,6 @@
4847
create_channel,
4948
on_channel_state_change,
5049
)
51-
from flwr.common.version import package_version as flwr_version
52-
from flwr.supercore.constant import APP_ID_PATTERN, APP_VERSION_PATTERN
5350

5451
from .auth_plugin import CliAuthPlugin, get_cli_plugin_class
5552
from .cli_account_auth_interceptor import CliAccountAuthInterceptor
@@ -547,94 +544,6 @@ def flwr_cli_grpc_exc_handler() -> Iterator[None]: # pylint: disable=too-many-b
547544
raise
548545

549546

550-
def request_download_link(
551-
app_id: str, app_version: str | None, in_url: str, out_url: str
552-
) -> str:
553-
"""Request a download link for the given app from the Flower platform API.
554-
555-
Parameters
556-
----------
557-
app_id : str
558-
The application identifier.
559-
app_version : str | None
560-
The application version, or None for latest.
561-
in_url : str
562-
The API endpoint URL.
563-
out_url : str
564-
The key name for the download URL in the response.
565-
566-
Returns
567-
-------
568-
str
569-
The download URL for the application.
570-
571-
Raises
572-
------
573-
typer.Exit
574-
If connection fails, app not found, or API request fails.
575-
"""
576-
headers = {
577-
"Content-Type": "application/json",
578-
"Accept": "application/json",
579-
}
580-
body = {
581-
"app_id": app_id, # send raw string of app_id
582-
"app_version": app_version,
583-
"flwr_version": flwr_version,
584-
}
585-
try:
586-
resp = requests.post(in_url, headers=headers, data=json.dumps(body), timeout=20)
587-
except requests.RequestException as e:
588-
typer.secho(
589-
f"Unable to connect to Platform API: {e}",
590-
fg=typer.colors.RED,
591-
err=True,
592-
)
593-
raise typer.Exit(code=1) from e
594-
595-
if resp.status_code == 404:
596-
error_message = resp.json()["detail"]
597-
if isinstance(error_message, dict):
598-
available_app_versions = error_message["available_app_versions"]
599-
available_versions_str = (
600-
", ".join(map(str, available_app_versions))
601-
if available_app_versions
602-
else "None"
603-
)
604-
typer.secho(
605-
f"{app_id}=={app_version} not found in Platform API. "
606-
f"Available app versions for {app_id}: {available_versions_str}",
607-
fg=typer.colors.RED,
608-
err=True,
609-
)
610-
else:
611-
typer.secho(
612-
f"{app_id} not found in Platform API.",
613-
fg=typer.colors.RED,
614-
err=True,
615-
)
616-
raise typer.Exit(code=1)
617-
618-
if not resp.ok:
619-
typer.secho(
620-
f"Platform API request failed with "
621-
f"status {resp.status_code}. Details: {resp.text}",
622-
fg=typer.colors.RED,
623-
err=True,
624-
)
625-
raise typer.Exit(code=1)
626-
627-
data = resp.json()
628-
if out_url not in data:
629-
typer.secho(
630-
"Invalid response from Platform API",
631-
fg=typer.colors.RED,
632-
err=True,
633-
)
634-
raise typer.Exit(code=1)
635-
return str(data[out_url])
636-
637-
638547
def build_pathspec(patterns: Iterable[str]) -> pathspec.PathSpec:
639548
"""Build a PathSpec from a list of GitIgnore-style patterns.
640549
@@ -711,49 +620,3 @@ def validate_credentials_content(creds_path: Path) -> str:
711620
raise typer.Exit(code=1)
712621

713622
return creds[ACCESS_TOKEN_KEY]
714-
715-
716-
def parse_app_spec(app_spec: str) -> tuple[str, str | None]:
717-
"""Parse app specification string into app ID and version.
718-
719-
Parameters
720-
----------
721-
app_spec : str
722-
The app specification string in the format '@account/app' or
723-
'@account/app==x.y.z' (digits only).
724-
725-
Returns
726-
-------
727-
tuple[str, str | None]
728-
A tuple containing the app ID and optional version.
729-
730-
Raises
731-
------
732-
typer.Exit
733-
If the app specification format is invalid.
734-
"""
735-
if "==" in app_spec:
736-
app_id, app_version = app_spec.split("==")
737-
738-
# Validate app version format
739-
if not re.match(APP_VERSION_PATTERN, app_version):
740-
typer.secho(
741-
"❌ Invalid app version. Expected format: x.y.z (digits only).",
742-
fg=typer.colors.RED,
743-
err=True,
744-
)
745-
raise typer.Exit(code=1)
746-
else:
747-
app_id = app_spec
748-
app_version = None
749-
750-
# Validate app_id format
751-
if not re.match(APP_ID_PATTERN, app_id):
752-
typer.secho(
753-
"❌ Invalid remote app ID. Expected format: '@account/app'.",
754-
fg=typer.colors.RED,
755-
err=True,
756-
)
757-
raise typer.Exit(code=1)
758-
759-
return app_id, app_version

framework/py/flwr/cli/utils_test.py

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,10 @@
2222
import unittest
2323
from pathlib import Path
2424

25-
import pytest
26-
import typer
27-
2825
from flwr.cli.utils import (
2926
build_pathspec,
3027
get_sha256_hash,
3128
load_gitignore_patterns,
32-
parse_app_spec,
3329
validate_credentials_content,
3430
)
3531
from flwr.common.constant import (
@@ -162,23 +158,3 @@ def test_load_gitignore_patterns_with_pathspec() -> None:
162158

163159
# Should not match normal files
164160
assert spec.match_file("good.py") is False
165-
166-
167-
@pytest.mark.parametrize(
168-
"value",
169-
[
170-
"user/app==1.2.3", # missing '@'
171-
"@accountapp==1.2.3", # missing slash
172-
"@account/app==1.2", # bad version
173-
"@account/app==1.2.3.4", # bad version
174-
"@account*/app==1.2.3", # bad user id chars
175-
"@account/app*==1.2.3", # bad app id chars
176-
],
177-
)
178-
def test_parse_app_spec_rejects_invalid_formats(value: str) -> None:
179-
"""For an invalid string, the function should fail fast with typer.Exit(code=1)."""
180-
with pytest.raises(typer.Exit) as exc:
181-
parse_app_spec(value)
182-
183-
# Ensure we specifically exited with code 1
184-
assert exc.value.exit_code == 1

framework/py/flwr/server/app.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,7 @@ def run_superlink() -> None:
324324
authz_plugin=authz_plugin,
325325
event_log_plugin=event_log_plugin,
326326
artifact_provider=artifact_provider,
327+
fleet_api_type=args.fleet_api_type,
327328
)
328329
grpc_servers = [control_server]
329330
bckg_threads: list[threading.Thread] = []

0 commit comments

Comments
 (0)