Skip to content

feat: add --attach/--detach options to fal deploy#1100

Merged
chamini2 merged 3 commits into
mainfrom
matteo/INFRA-385-13
Jul 6, 2026
Merged

feat: add --attach/--detach options to fal deploy#1100
chamini2 merged 3 commits into
mainfrom
matteo/INFRA-385-13

Merge branch 'main' into matteo/INFRA-385-13

085ef9d
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 6, 2026 in 9m 59s

Code review found 1 potential issue

Found 5 candidates, confirmed 1. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 1
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit projects/fal/src/fal/api/deploy.py:163-171 Misleading --strategy hint in attach validation error

Annotations

Check warning on line 171 in projects/fal/src/fal/api/deploy.py

See this annotation in the file changed.

@claude claude / Claude Code Review

Misleading --strategy hint in attach validation error

Minor UX nit: for app-name references (e.g. `fal deploy my-app --attach`), the CLI's `--strategy` is silently discarded by `_resolve_deployment_reference` — `get_app_data_from_toml` overwrites the AppData, and only `attach_to_deployment` is re-applied via `replace()`. If `pyproject.toml` sets `deployment_strategy = "recreate"`, users who run `fal deploy my-app --strategy rolling --attach` see this error telling them to "Use --strategy rolling" — which they already did. The "remove the attach fla