From ee9177087bbb6b91e4e62798fb469e9080f30fed Mon Sep 17 00:00:00 2001 From: badayvedat <54285744+badayvedat@users.noreply.github.com> Date: Fri, 26 Jun 2026 21:51:03 +0300 Subject: [PATCH] fix(fal): honor --strategy when deploying by app name --- projects/fal/src/fal/api/deploy.py | 14 +++++--- projects/fal/src/fal/cli/deploy.py | 5 +-- projects/fal/tests/unit/cli/test_deploy.py | 40 ++++++++++++++++++++++ 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/projects/fal/src/fal/api/deploy.py b/projects/fal/src/fal/api/deploy.py index 2fce2cfe7..3905779d4 100644 --- a/projects/fal/src/fal/api/deploy.py +++ b/projects/fal/src/fal/api/deploy.py @@ -112,7 +112,7 @@ def _resolve_deployment_reference( *, app_name: str | None = None, auth: AuthModeLiteral | None = None, - strategy: DeploymentStrategyLiteral = "rolling", + strategy: DeploymentStrategyLiteral | None = None, reset_scale: bool = False, ) -> tuple[tuple[str | None, str | None], AppData]: from fal.cli._utils import AppData, get_app_data_from_toml, is_app_name @@ -127,7 +127,7 @@ def _resolve_deployment_reference( app_data = AppData( auth=auth, - deployment_strategy=cast(DeploymentStrategyLiteral, strategy), + deployment_strategy=strategy, reset_scale=cast(bool, reset_scale), name=app_name, ) @@ -141,6 +141,12 @@ def _resolve_deployment_reference( assert resolved_app_name is not None app_data = get_app_data_from_toml(resolved_app_name) + + # re-apply the config user explicitly set so they take precedence over + # the TOML values. + if strategy is not None: + app_data = replace(app_data, deployment_strategy=strategy) + if app_data.python_entry_point is not None or app_data.ref is None: # python_entry_point is resolved by the loader; ref is None for # image-only apps. @@ -326,7 +332,7 @@ def prepare_deployment( *, app_name: str | None = None, auth: AuthModeLiteral | None = None, - strategy: DeploymentStrategyLiteral = "rolling", + strategy: DeploymentStrategyLiteral | None = None, reset_scale: bool = False, force_env_build: bool = False, environment_name: str | None = None, @@ -372,7 +378,7 @@ def deploy( *, app_name: str | None = None, auth: AuthModeLiteral | None = None, - strategy: DeploymentStrategyLiteral = "rolling", + strategy: DeploymentStrategyLiteral | None = None, reset_scale: bool = False, force_env_build: bool = False, environment_name: str | None = None, diff --git a/projects/fal/src/fal/cli/deploy.py b/projects/fal/src/fal/cli/deploy.py index 84adcf082..8cedd8e93 100644 --- a/projects/fal/src/fal/cli/deploy.py +++ b/projects/fal/src/fal/cli/deploy.py @@ -232,8 +232,9 @@ def valid_auth_option(option): parser.add_argument( "--strategy", choices=["recreate", "rolling"], - help="Deployment strategy.", - default="rolling", + help="Deployment strategy. Defaults to the value in pyproject.toml, " + "or 'rolling' if unset.", + default=None, ) parser.add_argument( "--no-scale", diff --git a/projects/fal/tests/unit/cli/test_deploy.py b/projects/fal/tests/unit/cli/test_deploy.py index 6b0a584de..96cd3901d 100644 --- a/projects/fal/tests/unit/cli/test_deploy.py +++ b/projects/fal/tests/unit/cli/test_deploy.py @@ -685,6 +685,46 @@ def test_deploy_with_toml_deployment_strategy( ) +@patch("fal.cli._utils.find_pyproject_toml", return_value="pyproject.toml") +@patch("fal.cli._utils.parse_pyproject_toml") +@patch("fal.api.deploy.execute_prepared_deployment") +@patch("fal.api.deploy._prepare_deployment_from_reference") +def test_deploy_cli_strategy_overrides_toml_for_app_name( + mock_prepare_ref, + mock_execute, + mock_parse_toml, + mock_find_toml, + mock_parse_pyproject_toml, +): + # Regression test: an explicit --strategy on the CLI must take precedence + # over the deployment_strategy set in pyproject.toml when deploying by app + # name. Previously the TOML value silently won and --strategy was dropped. + mock_parse_toml.return_value = mock_parse_pyproject_toml + + # my-app has deployment_strategy="rolling" in the TOML fixture. + args = mock_args(app_ref=("my-app", None), strategy="recreate") + + _deploy(args) + + project_root, _ = find_project_root(None) + + mock_prepare_ref.assert_called_once_with( + mock_prepare_ref.call_args[0][0], + (f"{project_root / 'src/my_app/inference.py'}", "MyApp"), + AppData( + ref=f"{project_root / 'src/my_app/inference.py'}::MyApp", + auth="shared", + deployment_strategy="recreate", + reset_scale=False, + team=None, + name="my-app", + local_project_root=".", + ), + force_env_build=False, + environment_name=None, + ) + + @patch("fal.cli._utils.find_pyproject_toml", return_value="pyproject.toml") @patch("fal.cli._utils.parse_pyproject_toml") @patch("fal.api.deploy.execute_prepared_deployment")