refactor(cli): extract drt run to commands/run.py — #573 Phase 2b PR (a)#579
Merged
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
… (a)) Moves the run command + its supporting infrastructure out of main.py: - LogFormat Enum (#577) - _STANDARD_LOG_FIELDS / _JsonFormatter / _configure_json_logging - _RunContext dataclass - _exit_code_for_signal (POSIX 128+signum) - _run_one (per-sync execution; observer composition; telemetry) - _print_watermark_summary (post-run notes) - run (@app.command itself; signal handling; parallel/sequential dispatch) Total ~485 LOC. cli/main.py shrinks 1086 → 595 (-45%). Back-compat: main.py re-exports each moved underscore-prefixed name + LogFormat so existing `from drt.cli.main import _RunContext` etc. keep working. Same pattern as #572's `_get_*` shims. Test mock paths updated for monkeypatched names that no longer live on drt.cli.main: - print_row_errors: was failing outright (AttributeError) — main.py no longer imports it after the move. - _get_destination / _get_watermark_storage: mocks were silently no-ops since #572 because _run_one calls the public _helpers.get_destination directly; updating the path so the mock is actually effective. Closes part of #573 (PR (b) covers validate / status / test). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds 5 unit tests in tests/unit/test_print_watermark_summary.py that exercise the helper directly: - empty results: no output - default_value branch: yellow first-run note + sync name list - cli_override branch: cyan --cursor-value note + sync name list - both branches: emits both notes - unknown / missing watermark_source: silent Lifts codecov patch coverage on commands/run.py from 90.27% to ~92% (22 → 18 missed lines). The remaining 18 lines are inside the run command body and need a CliRunner + project fixture to exercise; that's appropriate for a follow-up, since the watermark helper is the only piece reachable via direct call. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
masukai
force-pushed
the
refactor/573-cli-run-extract
branch
from
5905b2f to
4dcb750
Compare
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Summary
Phase 2b PR (a) of #546 (tracked under #573) — extracts the
drt runcommand + its supporting infrastructure out ofdrt/cli/main.pyinto a dedicateddrt/cli/commands/run.py. Pure mechanical move; no behaviour change.What moved
drt/cli/main.py→drt/cli/commands/run.py:LogFormat(str, Enum)--log-format_STANDARD_LOG_FIELDS,_JsonFormatter,_configure_json_loggingrunuses these)_RunContextdataclass_run_one_exit_code_for_signal_run_one_print_watermark_summaryrun(@app.command)Total moved: ~485 LOC.
drt/cli/main.pyshrinks 1086 → 595 LOC (-45%).Back-compat shim
drt/cli/main.pyre-exports each moved underscore-prefixed name +LogFormatso existing imports keep working:Same pattern as #572's
_get_source/_get_destination/_get_watermark_storageshims. A "tighten back-compat surface" pass is appropriate post-v1.0.Test mock paths updated
The move also corrects monkeypatch paths in
tests/unit/test_cli_run_telemetry.py:drt.cli.main._get_destinationdrt.cli.commands.run.get_destination_run_onecalls_helpers.get_destinationdirectly, not via the main.py shim). Fixed opportunistically.drt.cli.main._get_watermark_storagedrt.cli.commands.run.get_watermark_storagedrt.cli.main.print_row_errorsdrt.cli.commands.run.print_row_errorsAttributeError) after the move, sincemain.pyno longer importsprint_row_errors. The fix made it pass.All 10 tests in
test_cli_run_telemetry.pynow pass with the path adjustments and are actually effective mocks.Acceptance criteria
run+_RunContext+_run_one+_JsonFormattermoved todrt/cli/commands/run.pycli/main.pyshrinks (1086 → 595 LOC, -45%)LogFormatmake test: 1247 passed, 1 skippedmake lintclean (111 source files; ruff + mypy)drt run --helprenders identicallydrt run --log-format invalidstill rejected by Choice validation (Enum auto-derived)Out of scope (PR (b) follow-up)
validate(~190 LOC)status(~120 LOC)test+_SyncTestResult(~130 LOC)After PR (b),
cli/main.pywill hit the #573 target of ~50-100 LOC pure wiring.References
drt/cli/main.pyinto per-command handler modules #546)🤖 Generated with Claude Code