ENG-10963 refactor(log): hosting CLI shares the reflex-base console and logging (4/5) - #6866
Conversation
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.
Greptile SummaryThis PR consolidates hosting-CLI console and log-level behavior onto reflex-base and migrates diagnostic output to the shared standard-logging pipeline.
Confidence Score: 5/5The PR appears safe to merge, with no concrete changed-code defect identified. The shared LogLevel and console shims preserve hosting call compatibility, commands configure the shared logging pipeline before emitting migrated records, and the lockfile’s reported vulnerable dependency versions were not introduced or worsened by this change.
|
| Filename | Overview |
|---|---|
| packages/reflex-base/src/reflex_base/utils/console.py | Adds string-returning overloads for ask while retaining the existing runtime implementation. |
| packages/reflex-hosting-cli/src/reflex_cli/utils/console.py | Replaces the forked console implementation with reflex-base re-exports and a legacy-compatible string log-level adapter. |
| packages/reflex-hosting-cli/src/reflex_cli/constants/base.py | Re-exports the shared reflex-base LogLevel instead of maintaining a separate enum. |
| packages/reflex-hosting-cli/src/reflex_cli/utils/hosting.py | Routes hosting diagnostics through module logging and emits successful deployment states through the shared SUCCESS level. |
| packages/reflex-hosting-cli/src/reflex_cli/v2/apps.py | Migrates application-command diagnostics and success output to the shared logging pipeline without changing command control flow. |
| packages/reflex-hosting-cli/src/reflex_cli/v2/cli.py | Migrates login, logout, and deployment diagnostics to standard logging while preserving log-level initialization. |
| reflex/reflex.py | Passes the now-shared LogLevel directly to hosting commands and removes the temporary enum conversion helper. |
| packages/reflex-hosting-cli/pyproject.toml | Declares reflex-base as a runtime dependency and configures its local workspace source. |
| uv.lock | Updates workspace dependency metadata for reflex-hosting-cli without introducing the reported vulnerable package versions. |
Reviews (1): Last reviewed commit: "refactor(log): hosting CLI shares the re..." | 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_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>
<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>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ) | ||
|
|
||
| mock_console_error.assert_called_once_with("failed to retrieve secrets.") | ||
| errors = [r.getMessage() for r in caplog.records if r.levelno == logging.ERROR] |
There was a problem hiding this comment.
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.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/units/reflex_cli/v2/test_secrets.py, line 89:
<comment>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.</comment>
<file context>
@@ -79,7 +86,8 @@ def test_get_secrets_error(mocker: MockFixture):
)
- mock_console_error.assert_called_once_with("failed to retrieve secrets.")
+ errors = [r.getMessage() for r in caplog.records if r.levelno == logging.ERROR]
+ assert errors == ["failed to retrieve secrets."]
</file context>
| "httpx >=0.25.1,<1.0", | ||
| "packaging >=24.2", | ||
| "platformdirs >=3.10.0,<5.0", | ||
| "reflex-base >= 0.9.8.post19.dev0", |
There was a problem hiding this comment.
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.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-hosting-cli/pyproject.toml, line 21:
<comment>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.</comment>
<file context>
@@ -18,9 +18,13 @@ dependencies = [
"httpx >=0.25.1,<1.0",
"packaging >=24.2",
"platformdirs >=3.10.0,<5.0",
+ "reflex-base >= 0.9.8.post19.dev0",
"rich >=13,<16",
]
</file context>
| from reflex_cli.v2 import cli | ||
|
|
||
|
|
||
| def _log_messages(caplog: pytest.LogCaptureFixture, level: int) -> list[str]: |
There was a problem hiding this comment.
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.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/units/reflex_cli/v2/test_cli.py, line 18:
<comment>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.</comment>
<file context>
@@ -9,10 +10,24 @@
from reflex_cli.v2 import cli
+def _log_messages(caplog: pytest.LogCaptureFixture, level: int) -> list[str]:
+ """Return the captured log messages emitted at the given level.
+
</file context>
| assert result.exit_code == 0, result.output | ||
| mock_error.assert_called_once_with("get status failed: Invalid token") | ||
| errors = [r.getMessage() for r in caplog.records if r.levelno == logging.ERROR] | ||
| assert errors == ["get status failed: Invalid token"] |
There was a problem hiding this comment.
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.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/units/reflex_cli/v2/test_apps.py, line 365:
<comment>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.</comment>
<file context>
@@ -354,11 +361,17 @@ def test_deployment_status_http_error(mocker: MockFixture):
assert result.exit_code == 0, result.output
- mock_error.assert_called_once_with("get status failed: Invalid token")
+ errors = [r.getMessage() for r in caplog.records if r.levelno == logging.ERROR]
+ assert errors == ["get status failed: Invalid token"]
+
</file context>
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.