feat: report missing component dependencies with an actionable install hint - #262
feat: report missing component dependencies with an actionable install hint#262hasansezertasan wants to merge 1 commit into
Conversation
…l hint The CLI root lazy-imports each non-primary component. When a dependency is absent the failure was a bare ModuleNotFoundError traceback with no hint about the fix. Wrap every launcher's lazy import in a private `_component_dependencies` context manager that exits 1 with a message naming the component, the missing module, and `uv sync`. Component runtime deps are core `dependencies` (there are no per-component extras), so a missing module can only mean the environment is out of sync -- after a `copier update` that enabled a component, or in a stale venv -- which is why the hint is `uv sync` and not `pip install pkg[x]`. Emitted only when the root actually lazy-imports something, derived from `primary_component` per ADR-019, so a CLI-only project renders unchanged. The minimal launcher's default callback is covered too. Refs #172
|
Warning Review limit reached
Next review available in: 57 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 418f6374a8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| with _component_dependencies("web"): | ||
| from {{github_repo_name}}.web.app import main # noqa: PLC0415 | ||
|
|
||
| main() |
There was a problem hiding this comment.
Keep web startup inside the dependency guard
When uvicorn alone is missing, importing the web app succeeds because uvicorn is lazily imported inside web.app.main; this context has already exited before main() runs, so pkg web still emits the bare ModuleNotFoundError that this change is intended to replace. The same ordering exists in the argparse launcher and the minimal launcher's web-primary callback, and the new test misses it by raising directly inside the helper rather than invoking the actual launcher path.
Useful? React with 👍 / 👎.
Closes #172
What
The
pkgCLI root lazy-imports each non-primary component (pkg web,pkg worker, ...). When one of those imports hits a missing dependency, the failure was a bareModuleNotFoundErrortraceback with no hint about the fix.Every launcher command now wraps its lazy import in a private
_component_dependencies(component)context manager (both the Typer and argparse variants):A missing dependency becomes:
and exits 1 (
typer.Exit/SystemExit).Rescoped from the issue
#172 proposed a
core/_imports.pywith arequire(module, *, feature, extra)helper. Two findings from the code changed the design:[project.optional-dependencies]carries only an emptyall(kept so thedevgroup'spkg[all]resolves); every component runtime dep is a coredependency. Sopip install pkg[<extra>]can never be the right hint — a missing module means the environment is stale (acopier updatethat enabled a component without a re-sync, or a stale venv), anduv syncis the honest fix.from pkg.web.app import main), rendered under the same Jinja toggle that put the third-party dep independencies. After acopier updatethe first-party module is present; what fails is the third-party import one frame deeper, inside the component module. A context manager around the import site catches that; arequire()call replacing the import would not.So: no new
coremodule (nothing to add to the import-linter contract, no new coverage surface), no extras, and the guard is emitted only when the root actually lazy-imports something — derived fromprimary_componentper ADR-019, never re-spelled inline. A CLI-only project renders byte-identical to before. The minimal-launcher Typer root's default callback is covered too, which the issue didn't mention but has the same exposure.Verification
Seven shapes rendered with
--vcs-ref=HEAD— typer-multi, argparse-multi, minimal-launcher, cli-only, argparse-only, gui-primary, tui-gui:tox run -e stylegreen on all — ruff, mypy (3.10 + 3.14), basedpyright, ty, pyrefly, zuban, pylint, slotscheck, import-linter, taplo, typos, ec, sphinx-lint, dead-fixtures.tox runfull 3.10–3.14 matrix green.coverage combine && coverage report— 100%,fail_under = 99satisfied. The guard's error path is unit-tested (test_missing_component_dependency_is_actionable), so it carries no blanket pragma.mise run test— 118 passed; golden files unchanged (thelibrarypreset has no launcher components).prekclean on all changed files.Three defects the gates caught during development, all fixed: a
{{component}}f-string placeholder eaten by Jinja (surfaced only under the realtox run, not a quick editablepytest), basedpyrightreportImplicitStringConcatenationon the message built with+ PROJECT_NAME +, andreportDeprecatedonIteratorunder@contextmanager(nowGenerator[None, None, None], valid on the 3.10 floor).Unrelated finding
On an untagged fresh render, hatch-vcs writes a
src/<pkg>/_version.pythat trips ruff'sunsorted-dunder-allandbad-quotes-inline-stringintox -e style. It reproduces identically on a baseline render from the released tag, without this change — pre-existing, worth its own issue if not already known.