-
Notifications
You must be signed in to change notification settings - Fork 691
feat: Support python -m dynamo.frontend --version
#2449
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds per-component version generation via a Hatch build hook that writes _version.py files, exposes version at package level across components, and introduces a --version CLI flag for multiple backends and tools. Also updates pyproject.toml to register the hook and adjusts .gitignore and some headers. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Hatch as Hatch Build
participant Hook as VersionWriterHook
participant FS as Filesystem
Dev->>Hatch: build
Hatch->>Hook: initialize(version, build_data)
Hook->>Hook: compute full_version (+ git short hash if available)
loop for each component
Hook->>FS: write components/.../_version.py (__version__ = full_version)
end
Hook-->>Hatch: done
Hatch-->>Dev: build artifacts
sequenceDiagram
participant User
participant CLI as Component CLI
participant Pkg as dynamo.<component> (__init__)
User->>CLI: run ... --version
CLI->>Pkg: import __version__
CLI-->>User: print "Dynamo <Component> {__version__}" and exit
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Nitpick comments (13)
.gitignore (1)
95-95: Remove duplicate .build/ ignore
.build/is already ignored at Line 90. Keep a single occurrence to avoid noise.Apply this diff:
-# Local build artifacts for devcontainer -.build/ +# Local build artifacts for devcontainerhatch_build.py (1)
47-52: Ensure directory exists and write with explicit encodingA small hardening: make sure parent directories exist to future-proof against path changes, and specify UTF-8 encoding.
Apply this diff:
for component in COMPONENTS: version_file_path = os.path.join( self.root, f"components/{component}/_version.py" ) - with open(version_file_path, "w") as f: + os.makedirs(os.path.dirname(version_file_path), exist_ok=True) + with open(version_file_path, "w", encoding="utf-8") as f: f.write(version_content)components/backends/vllm/src/dynamo/vllm/args.py (1)
15-16: Avoid importing package init just to read version; fetch distribution version via importlib.metadata insteadThis decouples CLI version reporting from package import-time behavior and avoids potential issues if
_version.pyisn’t present yet.Apply this diff:
-from dynamo.vllm import __version__ +from importlib.metadata import version as _pkg_version- parser.add_argument( - "--version", action="version", version=f"Dynamo Backend VLLM {__version__}" - ) + parser.add_argument( + "--version", + action="version", + version=f"Dynamo Backend VLLM {_pkg_version('ai-dynamo')}", + )Optionally, if you prefer to avoid calling
_pkg_versionin an f-string, compute once:version_str = _pkg_version("ai-dynamo") parser.add_argument("--version", action="version", version=f"Dynamo Backend VLLM {version_str}")Also applies to: 63-65
components/backends/vllm/src/dynamo/vllm/__init__.py (1)
4-6: Drop redundant reassignment and add a safe fallback when _version.py is absent (editable/dev installs).
__version__ = __version__is a no-op and can be removed.- If
_version.pyisn’t present (e.g., running from source without the Hatch hook), importing will raise. A small fallback keeps imports resilient.Proposed change:
-from ._version import __version__ - -__version__ = __version__ +try: + from ._version import __version__ # generated at build time by hatch hook +except Exception: + __version__ = "0.0.0+unknown"Would you like me to apply the same pattern across all component init.py files for consistency?
components/backends/trtllm/src/dynamo/trtllm/utils/trtllm_utils.py (1)
9-9: Top-level version import can fail in editable/dev installs. Consider deferring or ensuring package-level fallback.Importing
__version__at module import time will fail if_version.pywasn’t generated yet. If you adopt the safe-fallback pattern indynamo.trtllm.__init__, this becomes safe. Alternatively, defer the import insidecmd_line_args()to reduce import-time coupling.If you prefer deferring, this minimal change works:
-from dynamo.trtllm import __version__ @@ def cmd_line_args(): - parser = argparse.ArgumentParser( + parser = argparse.ArgumentParser( description="TensorRT-LLM server integrated with Dynamo LLM." ) + # Defer import to avoid import-time dependency on generated _version.py + try: + from dynamo.trtllm import __version__ as _ver + except Exception: + _ver = "0.0.0+unknown" parser.add_argument( - "--version", action="version", version=f"Dynamo Backend TRTLLM {__version__}" + "--version", action="version", version=f"Dynamo Backend TRTLLM {_ver}" )components/planner/src/dynamo/planner/utils/planner_core.py (2)
12-12: Importing version from the package is fine; ensure init stays lightweight.If
dynamo.planner.__init__remains minimal (only version re-export), this is safe. If it grows, consider importing from. _versiondirectly or using a safe fallback pattern in__init__to avoid import-time surprises.
337-339: Good addition: --version flag for the planner CLI.Conventional usage with argparse’s version action; consistent with other components.
Optional: add a short alias
-Vfor parity with common CLIs:- parser.add_argument( - "--version", action="version", version=f"Dynamo Planner {__version__}" - ) + parser.add_argument( + "--version", "-V", action="version", version=f"Dynamo Planner {__version__}" + )components/backends/mocker/src/dynamo/mocker/main.py (2)
12-12: Top-level version import note.Same consideration as other components: this import is fine if
dynamo.mocker.__init__is resilient when_version.pyis absent. If not, consider the safe-fallback pattern in the package__init__or defer the import withincmd_line_args().
45-47: LGTM: standard --version support via argparse.Consistent with repo-wide pattern. Behavior is correct and self-contained.
Optional: add a
-Valias:- parser.add_argument( - "--version", action="version", version=f"Dynamo Mocker {__version__}" - ) + parser.add_argument( + "--version", "-V", action="version", version=f"Dynamo Mocker {__version__}" + )components/backends/mocker/src/dynamo/mocker/__init__.py (1)
4-6: Avoid no-op reassignment and make version import resilient.
__version__ = __version__doesn’t add value.- Provide a runtime fallback for source checkouts without generated
_version.py.Suggested change:
-from ._version import __version__ - -__version__ = __version__ +try: + from ._version import __version__ # generated at build time by hatch hook +except Exception: + __version__ = "0.0.0+unknown"If you agree, I can apply the same change across all component packages for consistency.
components/frontend/src/dynamo/frontend/main.py (1)
75-77: Nice: Standard argparse --version behavior wired correctly.
action="version"will print and exit before validating other args, which is what we want forpython -m dynamo.frontend --version. Format looks consistent with the project.Optional: Consider
version=f"Dynamo Frontend {__version__}"->version=f"%(prog)s {__version__}"if you prefer the program name to reflect the actual entry point. Current explicit label is fine if that’s the intended branding.components/backends/llama_cpp/src/dynamo/llama_cpp/main.py (1)
14-14: Prefer a relative import for version to avoid package-level coupling.This keeps the CLI insulated from potential
__init__changes and ensures we read the generated version directly.Apply this diff:
-from dynamo.llama_cpp import __version__ +from ._version import __version__If you want to keep importing from the package for consistency, verify that
dynamo.llama_cpp.__init__re-exports__version__everywhere this is used.components/backends/llama_cpp/src/dynamo/llama_cpp/__init__.py (1)
6-6: Optionally make the re-export explicitIf you want to make the public API explicit and silence some linters about re-exports, declare all.
-__version__ = __version__ +__version__ = __version__ +__all__ = ["__version__"]
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (16)
.gitignore(2 hunks)components/backends/llama_cpp/src/dynamo/llama_cpp/__init__.py(1 hunks)components/backends/llama_cpp/src/dynamo/llama_cpp/main.py(2 hunks)components/backends/mocker/src/dynamo/mocker/__init__.py(1 hunks)components/backends/mocker/src/dynamo/mocker/main.py(2 hunks)components/backends/sglang/src/dynamo/sglang/__init__.py(1 hunks)components/backends/trtllm/src/dynamo/trtllm/__init__.py(1 hunks)components/backends/trtllm/src/dynamo/trtllm/utils/trtllm_utils.py(2 hunks)components/backends/vllm/src/dynamo/vllm/__init__.py(1 hunks)components/backends/vllm/src/dynamo/vllm/args.py(2 hunks)components/frontend/src/dynamo/frontend/__init__.py(1 hunks)components/frontend/src/dynamo/frontend/main.py(2 hunks)components/planner/src/dynamo/planner/__init__.py(1 hunks)components/planner/src/dynamo/planner/utils/planner_core.py(2 hunks)hatch_build.py(1 hunks)pyproject.toml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: pre-merge-rust (.)
- GitHub Check: pre-merge-rust (lib/bindings/python)
- GitHub Check: Build and Test - dynamo
🔇 Additional comments (6)
pyproject.toml (1)
77-79: Hatch custom build hook registration looks correctHook path and placement are valid for Hatchling, enabling version file generation during builds.
hatch_build.py (1)
30-44: Git hash suffix logic is sound and resilientGracefully appends the short commit hash when available and falls back cleanly otherwise. PEP 440 local version formatting with “+” is appropriate.
components/backends/vllm/src/dynamo/vllm/args.py (1)
63-65: Check for duplicate--versionacross CLIs — verify upstream vLLM doesn't register--versionRepo search found multiple local argparse
--versionregistrations; a duplicate-option error only happens if the same parser gets two registrations (e.g., if upstream vLLM registers args on import). Confirm vLLM’s import-time behavior; if it does register--version, guard this add.Found occurrences:
- components/planner/src/dynamo/planner/utils/planner_core.py:338
- components/frontend/src/dynamo/frontend/main.py:76
- components/backends/trtllm/src/dynamo/trtllm/utils/trtllm_utils.py:114
- components/backends/mocker/src/dynamo/mocker/main.py:46
- components/backends/vllm/src/dynamo/vllm/args.py:64 <-- file under review
- components/backends/llama_cpp/src/dynamo/llama_cpp/main.py:88
Original snippet (unchanged):
parser.add_argument( "--version", action="version", version=f"Dynamo Backend VLLM {__version__}" )Suggested quick guard if upstream does add
--version:if "--version" not in parser._option_string_actions: parser.add_argument("--version", action="version", version=f"Dynamo Backend VLLM {__version__}")components/backends/trtllm/src/dynamo/trtllm/utils/trtllm_utils.py (1)
113-115: Nice: clean argparse --version integration.Using
action="version"ensures proper printing and immediate exit, keeping CLI behavior conventional.components/backends/llama_cpp/src/dynamo/llama_cpp/main.py (1)
87-89: LGTM: --version integrates cleanly with required args.Argparse’s version action exits before validating
--model-path, sopython -m dynamo.llama_cpp --versionworks without extra flags. Message format matches the pattern used elsewhere.components/backends/llama_cpp/src/dynamo/llama_cpp/__init__.py (1)
1-2: SPDX headers: LGTMLicense headers look correct and consistent.
Version looks like this `0.4.0+3a3f5bf2` if you have git, `0.4.0` otherwise. The `--version` flag should work on all components except sglang, because that handles arguments differently. All components (including sglang) have the expected `__version__` attribute at the top level.
4581aaa to
94125d0
Compare
| description="llama.cpp server integrated with Dynamo LLM." | ||
| ) | ||
| parser.add_argument( | ||
| "--version", action="version", version=f"Dynamo Backend llama.cpp {__version__}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Woah, I have written many argparse CLIs, and today I learned that action="version" was a thing! Thanks @grahamking 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's neat right?! It stops parsing when it sees that, prints the version and exits, no extra code required.
I take not credit because that was Claude Code. I gave it a spec and it did the design (hatch hook that writes a _version.py) and initial draft. It's hatch hook used an old API, so I discussed it with GPT-5 which gave me the new hook and correct way to read the build version. Then Code Rabbit reviewed it and I re-worked the PR based on it's feedback to use importlib as a fallback. I beautiful symphony of human and AI.
Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
Version looks like this
0.4.0+3a3f5bf2if you have git,0.4.0otherwise.The
--versionflag should work on all components except sglang, because that handles arguments differently.All components (including sglang) have the expected
__version__attribute at the top level.Summary by CodeRabbit
New Features
Chores