ENG-11157: wait out "being scaled" instead of failing the deploy - #6886
Draft
Kastier1 wants to merge 7 commits into
Draft
ENG-11157: wait out "being scaled" instead of failing the deploy#6886Kastier1 wants to merge 7 commits into
Kastier1 wants to merge 7 commits into
Conversation
Adds first-class autoscaling controls to `reflex deploy`, backed by the instance bounds route merged server side in flexgen #4744. New `hosting.set_instance_bounds()` posts to /api/v1/apps/{app_id}/instance_bounds with only the bounds the user actually passed, so apps that don't override keep their platform defaults. It runs after the app is resolved and validated but before the export and the deployment submit: the deployment reads the bounds off the app when it is created, and failing before the export means a rejected bound doesn't cost a full build. Server details for the 400 (validation / unsupported platform) and 409 (scale in progress) cases are surfaced verbatim rather than pre-validated in the CLI. Also adds regression coverage for the existing --vmtype passthrough, pinning it from the CLI flag through to the `vm_type` submit field. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The server has honored GCP sizing since flexgen#4744; this block predates that and silently dropped the flag — the same gap the customer originally reported. Only --region is still dropped (it comes from the connection). Verified end to end against staging: the flag reaches the deploy submit, the deployment records Single CPU Medium, and the Cloud Run revision rolls out with cpu:1/memory:1024Mi. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Move set_instance_bounds() from before the export to immediately before create_deployment(). Applying it earlier meant a failed export left the new bounds on the app with no deployment to consume them, silently changing what the *next* deployment scales to — and, for a non-zero minimum, what the app costs to run. The residual window is now just the submit itself, and if that is rejected the CLI says the bounds stuck instead of letting the next deploy inherit them unannounced. Append min_instances/max_instances to the end of deploy()'s signature rather than grouping them next to vmtype, so positional callers keep binding hostname and everything after it to the same parameters. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The "bounds outlived the deploy" warning only fired when create_deployment returned a failure string. It raises on transport errors, and Ctrl-C during the submit is a third exit, so both left the bounds changed silently. Move the warning into a _warn_if_bounds_outlive_deploy context manager wrapping the submit, so every path out of it reports. mock_export_import_error_fn took a fixed 6 arguments while deploy() calls export_fn with 7 on reflex > 0.7.6, so it raised TypeError, not the ImportError its name promises. Both tests using it still passed, for the wrong reason. Take *args so the intended path is the one exercised. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
set_instance_bounds raises on a transport failure, which happens before the warning context around the submit is entered. A response lost after the server applied the write therefore left billable state changed with no message at all -- the one gap in "a failed deploy always says what stuck". Handle it at the call site rather than in the helper: hosting.py's convention is that transport errors propagate, and deploy() is what knows this particular call has persistent, billable side effects. The message hedges both ways, since a dropped connection says nothing about whether the write landed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Also add the missing hosting-cli bugfix fragment for the --vmtype passthrough on Google Cloud deploys, which shipped in 5a70527 without one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both refusals are short-lived — usually the caller's own previous action still rolling out, and the bounds call itself re-rolls a running app so the submit right behind it meets the same marker. Retry each with a 15s interval (2-3 minutes total) before giving up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Greptile SummaryThe PR makes deployments tolerate transient app-scaling conflicts by retrying instance-bound updates and deployment submission, while adding autoscaling bounds to the top-level deploy command.
Confidence Score: 5/5The PR appears safe to merge with no concrete blocking or independently actionable non-blocking issue established. The retry loops are bounded, preserve existing deployment error handling, and target the production scaling phrase documented in the PR; bounds failures and uncertain writes are surfaced without silently continuing deployment.
|
| Filename | Overview |
|---|---|
| packages/reflex-hosting-cli/src/reflex_cli/v2/cli.py | Adds bounded retries around scaling conflicts, applies instance bounds immediately before submission, preserves GCP VM types, and warns when bounds survive a failed deploy. |
| packages/reflex-hosting-cli/src/reflex_cli/utils/hosting.py | Adds the authenticated instance-bounds API call and propagates structured HTTP error details to the deploy flow. |
| reflex/reflex.py | Exposes optional minimum and maximum instance flags and forwards them to the hosting CLI. |
| tests/units/reflex_cli/v2/test_cli.py | Covers bounds ordering, omission, API rejection, uncertain writes, failed submission warnings, and GCP VM-type forwarding. |
| tests/units/reflex_cli/utils/test_hosting.py | Covers partial bounds request bodies, API errors, authentication headers, and VM-type submission data. |
Reviews (1): Last reviewed commit: "ENG-11157: wait out "being scaled" inste..." | Re-trigger Greptile
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #6884. Both "being scaled" refusals during
reflex deployare short-lived — usually the caller's own previous action still rolling out, and the instance-bounds call itself re-rolls a running app, so the submit right behind it meets the same scaling marker. The CLI now retries each at 15s intervals (~2–3 minutes total) with a progress message before giving up, instead of failing a deploy the user will immediately rerun.Observed live against staging:
reflex deploy --min-instances 2failed withset instance bounds failed: the app is being scaled; change instance bounds after it finisheswhenever the previous roll-out hadn't settled.Linear: ENG-11157
🤖 Generated with Claude Code