ENG-10963 refactor(log): hosting CLI shares the reflex-base console and logging (4/5) - #6866
Conversation
Greptile SummaryThe PR consolidates hosting CLI console and logging behavior onto reflex-base and removes the temporary LogLevel conversion layer.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains in the eligible follow-up review scope. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/reflex-base/src/reflex_base/utils/console.py | Adds string-returning ask overloads and configurable table-cell overflow while preserving the shared console implementation. |
| packages/reflex-hosting-cli/src/reflex_cli/utils/console.py | Replaces the forked console implementation with reflex-base re-exports and a legacy string-compatible log-level wrapper. |
| packages/reflex-hosting-cli/src/reflex_cli/constants/base.py | Re-exports reflex-base's LogLevel so Reflex and the hosting CLI use the same enum class. |
| packages/reflex-hosting-cli/src/reflex_cli/utils/hosting.py | Migrates user diagnostics and deployment status messages from the local console helpers to standard logging. |
| packages/reflex-hosting-cli/src/reflex_cli/v2/cli.py | Migrates command diagnostics to standard logging and uses the shared SUCCESS level for successful operations. |
| packages/reflex-hosting-cli/src/reflex_cli/v2/apps.py | Routes application-management diagnostics through module logging while retaining direct console output for requested command results. |
| packages/reflex-hosting-cli/pyproject.toml | Adds the reflex-base runtime dependency and configures its workspace source. |
| reflex/reflex.py | Passes the now-shared LogLevel directly to hosting commands and removes the temporary enum conversion mapper. |
| uv.lock | Updates the resolved workspace dependency graph to include reflex-base for reflex-hosting-cli. |
Reviews (13): Last reviewed commit: "chore(news): name the hosting CLI fragme..." | Re-trigger Greptile
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
4 issues found across 27 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid β if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/units/reflex_cli/v2/test_cli.py">
<violation number="1" location="tests/units/reflex_cli/v2/test_cli.py:18">
P2: The caplog-based assertions in the migrated tests may not capture the records they expect. caplog listens on the root logger, so it only sees records that propagate up the logger hierarchy. But cli.logout/deploy call `console.set_log_level(...)` before emitting the messages under test, and that call configures the reflex-base pipeline (reflex_base/utils/log.py `configure()`), which sets `logger.propagate = False` on the `reflex_cli` root logger and attaches its own handler. After propagation is disabled, the `logger.error`/`logger.log(SUCCESS)` records are handled by the reflex_cli handler and never reach caplog, so these assertions can return empty. Worth verifying these tests actually pass in the full suite (order-dependently they may fail once any earlier test configures the pipeline), and if so, capture at the `reflex_cli` logger level or assert via the reflex-base handler instead of the root-level caplog.</violation>
</file>
<file name="tests/units/reflex_cli/v2/test_apps.py">
<violation number="1" location="tests/units/reflex_cli/v2/test_apps.py:365">
P2: These new assertions read records off the `caplog` fixture, which captures by attaching a handler to the root logger. But the shared reflex-base logging pipeline (on which this PR switches the hosting CLI) sets `logger.propagate = False` on the `reflex_cli` package-root logger (and `reflex_base`), so records emitted by `reflex_cli.v2.apps` never reach the root logger. If the pipeline's `bootstrap()`/`configure()` runs during the test (the commands call `console.set_log_level`, which routes into reflex_base logging), `caplog.records` will be empty and the strict equality asserts like `errors == ["get status failed: Invalid token"]` / `warnings == ["..."]` will fail (or, with `in`/`[-1]` checks, pass vacuously). Consider capturing from the `reflex_cli` logger directly (e.g. `caplog.set_level(..., logger="reflex_cli")`), or verify in the test env that propagation is not disabled, so the assertions actually observe the records.</violation>
</file>
<file name="tests/units/reflex_cli/v2/test_secrets.py">
<violation number="1" location="tests/units/reflex_cli/v2/test_secrets.py:89">
P1: These new caplog assertions will never capture the log records: the reflex logging pipeline sets propagate=False on the reflex_cli root logger in configure() (triggered by every command via console.set_log_level), so records on reflex_cli.v2.secrets don't reach the root logger where caplog attaches its handler. caplog.records will be empty and the tests fail. Capture at the reflex_cli logger instead (e.g. caplog.at_level / attach a handler to logging.getLogger('reflex_cli')) or assert on the rendered output.</violation>
</file>
<file name="packages/reflex-hosting-cli/pyproject.toml">
<violation number="1" location="packages/reflex-hosting-cli/pyproject.toml:21">
P2: The new `reflex-base >= 0.9.8.post19.dev0` dependency is an unpublishable `.dev` pin. The repo's publish gate (`uv run python scripts/check_min_deps.py --check-dev-pins`) fails for any package that declares a `*.dev` dependency pin, and this minimum will not be satisfiable from PyPI once reflex-hosting-cli is published (it only resolves locally through the added `[tool.uv.sources]` workspace entry). This is acknowledged as temporary in the PR description (re-pin at the 0.9.9 release), but it will block the hosting CLI publish gate in the meantime β worth tracking so it isn't merged/released with the dev pin still in place.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
963fc1b to
36b682d
Compare
8aed474 to
7b88585
Compare
8f5a51b to
af9cfbe
Compare
af9cfbe to
ff238bf
Compare
ff238bf to
14dc4c8
Compare
24c396d to
8f6c5f8
Compare
reflex-hosting-cli now depends on reflex-base: its forked console module and LogLevel enum become shims over the shared ones, and its logging goes through standard python logging. The base console.ask gains the same str-typed overloads the forked console had. Debug output renders purple (was blue), errors go to stderr (was stdout), and success messages are hidden at --loglevel warning. The now-redundant LogLevel conversion mapper in reflex/reflex.py is removed.
8f6c5f8 to
bc90a38
Compare
* fix(hosting-cli): make reflex-base an optional dependency reflex-hosting-cli declared `reflex-base >= 0.9.8.post19.dev0` while advertising support for reflex down to 0.6.6.post1. reflex-base does not exist below 0.9.0, so that floor could not be satisfied on any reflex the CLI claims to support, and on reflex 0.8.9 (which declares no reflex-base dependency) pip would silently install a second, mismatched framework base alongside it with no conflict to detect. The dependency was never needed for the shared logging pipeline: reflex_base.utils.log lists "reflex_cli" in PACKAGE_LOGGER_NAMES, so it parents the CLI's loggers from its own side. Only three things were genuinely imported β LogLevel, log.SUCCESS, and the rich console helpers β and each is now resolved from reflex-base when it is importable and from a local fork otherwise. On reflex 0.9 every name resolves to the identical reflex-base object, so behavior there is unchanged. The forked LogLevel moves to reflex_cli.constants.log_level so it is type-checked on its own, and reflex_cli.utils.log holds the fallback SUCCESS level and rich handler (same styles, stderr routing and level gating as reflex-base). reflex_cli.utils.console drops the PoorProgress, is_debug and debug/info/success/log/warn/error/timing/deprecate re-exports rather than duplicate them into the fallback: the CLI has logged through `logging` since #6866, and reflex-base deprecated the legacy helpers in #6867. * test(hosting-cli): guard the advertised minimum reflex version The runtime half was already covered: test_cli_imports_without_reflex_base fails if a CLI module imports a workspace package older reflex does not ship. Nothing covered the packaging half β re-adding `reflex-base` to the declared dependencies without importing it at module scope broke no test. Three interlocking guards, derived from the checkout rather than hard-coded so a package added under packages/ later is covered automatically: - no declared dependency on a workspace package while MINIMUM_REFLEX_VERSION predates the workspace split (reflex 0.9.0, where reflex-base first shipped) - no [tool.uv.sources] workspace entry, which resolves locally and so hides an unsatisfiable dependency inside the monorepo - MINIMUM_REFLEX_VERSION <= RECOMMENDED_REFLEX_VERSION, so the CLI cannot gate on a version it then tells the user to upgrade away from The first guard skips itself if the floor is raised past the split, at which point a workspace dependency is legitimate β and the third then catches the half-migration of raising the minimum without the recommendation. * fix(hosting-cli): complete the forked LogLevel API and stop test state leaking Review feedback on #6939. - The forked LogLevel was missing subprocess_level(), which the shared enum defines. Nothing calls it on the CLI's enum today (only reflex calls it, on its own config.loglevel), but parity is the fork's whole contract: a method present on reflex 0.9 and absent on older reflex is precisely the bug class this package guards against. Added it, and added a test asserting the fork covers reflex-base's entire public LogLevel API so the next gap fails loudly rather than waiting to be noticed. - test_set_log_level_accepts_strings exercised the reflex-base path outside the isolation context. That path is process-wide -- it sets REFLEX_LOGLEVEL so subprocesses inherit the level, and moves a module global -- so the test left REFLEX_LOGLEVEL=info behind for the rest of the session. Sandboxed the env var with monkeypatch and restore the previous level in a finally. - codespell: "re-use" -> "reuse". * test(hosting-cli): ban only the framework packages, not every workspace member Review feedback on #6939. The guard treated every distribution under packages/ as unavailable at the advertised reflex floor, which is too broad: reflex-release and reflex-docgen are ordinary PyPI distributions that install fine against any reflex, so depending on one would have failed the test for no reason. The set that actually matters is the framework runtime -- the workspace packages reflex itself depends on. Their presence and version are decided by whichever reflex the user installed, which is what makes a CLI dependency on one either unsatisfiable or a silent second copy. That set is now derived by intersecting the root package's dependencies with the workspace members (minus the hosting CLI itself), so it keeps tracking the checkout. The discoverability test now pins both directions: reflex-base and reflex-components-core are in the banned set, reflex-release and reflex-docgen are not.
reflex-hosting-cli now depends on reflex-base: its forked console module and
LogLevelenum become shims over the shared ones, and its logging goes through standard pythonlogging.console.askgains the same str-typed overloads the forked console had (the flatstr | Nonesignature broke typing at the hosting call sites).--loglevel warning.LogLevelconversion mapper inreflex/reflex.pyis removed.reflex-base >= 0.9.8.post19.dev0) must be re-pinned at the 0.9.9 release.Stack (ENG-10963)
#6863 β #6864 β #6865 β this β deprecate.
Merge in order; each PR is based on the previous branch.