Repository navigation
fix(hosting-cli): fail cloud config when PyYAML is missing - #7670
gaurav0107 wants to merge 1 commit into
Conversation
`generate_config` logged "Please install PyYAML" and returned None, the
same value it returns when a cloud.yml already exists. `reflex cloud
config --json` therefore printed {"generated": false, "path": null} and
exited 0, so a caller trusting the exit code read a missing dependency
as "nothing to do".
Raise `click.exceptions.Exit(1)` instead, so the command exits non-zero
and, with --json, writes no document -- the same contract the regions
and vmtypes commands follow for a failed read. The existing-file case
keeps returning None and exiting 0.
[Low impact] The PR appears safe to merge, with no actionable issues found. Summary
Review checks were limited to reading code; tests were not run. Reviews (1) · Last reviewed commit: "fix(hosting-cli): fail cloud config when..." · Reviewed by Greptile |
Merging this PR will improve performance by 4.13%2 warnings
Performance Changes
Tip Curious why performance improved? Comment Comparing |
All Submissions:
Type of change
Description
Closes #7092.
On
main,regionsandvmtypesalready exit 1 and write no JSON document when the request fails (#6917 / #7207). The case from the issue that is still open isreflex cloud config --jsonwithout PyYAML.hosting.generate_configlogged "Please install PyYAML to use this command" and returnedNone. That is the same value it returns when acloud.ymlalready exists, so the command printed{"generated": false, "path": null}and exited 0.This PR makes
generate_configraiseclick.exceptions.Exit(1)after logging that error. The command now exits 1, and with--jsonit writes nothing to stdout, which is howregions/vmtypeshandle a failed read. The check still runs before authentication, so no token is requested.I left the existing-
cloud.ymlcase alone. It still returnsNoneand exits 0 withgenerated: false, astest_generate_cloud_config_json_output_when_nothing_writtenrequires.One behaviour change: plain
reflex cloud config(without--json) also exits 1 now when PyYAML is missing, where it used to exit 0. The news fragment mentions this.News fragment:
packages/reflex-hosting-cli/news/+config-missing-pyyaml.bugfix.md.Changes To Core Features:
Validation
Tests (each one makes PyYAML unimportable with
mocker.patch.dict("sys.modules", {"yaml": None})):tests/units/reflex_cli/v2/test_vmtypes_regions.pytest_generate_cloud_config_json_without_pyyaml_writes_no_document: exit 1, empty stdout, no auth call.test_generate_cloud_config_without_pyyaml_exits_nonzero: exit 1, the PyYAML error is logged at ERROR, nothing printed.tests/units/reflex_cli/utils/test_hosting.pytest_generate_config_without_pyyaml_raises:Exit(1), no auth call.test_generate_config_with_existing_file_returns_none: an existingcloud.ymlis left unchanged andNoneis returned.On
main, the three PyYAML tests fail (the command exits 0 andgenerate_configdoes not raise). With this change,uv run pytest tests/units/reflex_clipasses (804 tests).ruff check,ruff format --check, and pre-commit (pyright, ty, codespell) pass on the changed files. I did not run the fulltests/unitssuite locally; CI will.🤖 Opened with Superhuman, an open-source contribution agent.