Implement jmp driver list command for listing locally installed drivers
#258
Conversation
WalkthroughThis pull request introduces a new asynchronous CLI for the Jumpstarter driver, adding a dedicated driver command group with commands such as Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CLI as Jumpstarter CLI
participant DC as Driver Command Group
participant EP as Entry Points & Table Builder
U->>CLI: Run "jmp driver list_drivers"
CLI->>DC: Execute driver command group
DC->>EP: Retrieve available driver entry points
EP-->>DC: Return driver data table
DC->>CLI: Output formatted driver list
CLI-->>U: Display driver list result
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Humm, I'm not sure how these import semantics look. A bit too confusing I think... |
NickCao
changed the title
jmp driver list command for listing locally installed drivers
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/jumpstarter-cli-driver/jumpstarter_cli_driver/driver.py (1)
7-8: Remove unnecessary async.The
list_driversfunction is marked as async but doesn't perform any asynchronous operations. Consider removing the async keyword to improve clarity.-@click.command("list") -async def list_drivers(): +@click.command("list") +def list_drivers():packages/jumpstarter-cli-driver/jumpstarter_cli_driver/__init__.py (1)
10-17: Enhance command group docstring.The current docstring could be more descriptive to help users understand the command's purpose and available subcommands.
@click.group(cls=AliasedGroup) @opt_log_level def driver(log_level: Optional[str]): - """Jumpstarter driver CLI tool""" + """ + Jumpstarter driver CLI tool. + + Commands: + list List locally installed drivers + version Show version information + """packages/jumpstarter-cli-driver/pyproject.toml (1)
26-28: Script Entry Point with Import ConsiderationThe
project.scriptsentry (jmp-driver = "jumpstarter_cli_driver:driver") correctly exposes the CLI command. However, given past comments about confusing import semantics, please double-check that thedrivercallable in thejumpstarter_cli_drivermodule is imported and documented unambiguously. A clarifying docstring or inline comment could help other developers understand the resolution of the import.packages/jumpstarter-driver-dutlink/pyproject.toml (1)
25-30: New Dutlink Driver Entry PointsThe newly added entry points for
Dutlink,DutlinkSerial,DutlinkStorageMux, andDutlinkPowerunder[project.entry-points."jumpstarter.drivers"]are set up correctly. Please verify that the corresponding classes injumpstarter_driver_dutlink.driverare implemented as intended.pyproject.toml (1)
10-10: New Workspace Entry Addition: Ensure Consistent Module IntegrationThe new entry
jumpstarter-cli-driver = { workspace = true }is correctly added in the[tool.uv.sources]section. This aligns with the existing workspace configuration and integrates the newjumpstarter-cli-drivermodule as intended for the CLI enhancements introduced in this PR. Please ensure that the module itself follows the same import and dependency patterns as the other CLI modules to avoid any potential confusion regarding import semantics.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
packages/jumpstarter-cli-driver/README.md(1 hunks)packages/jumpstarter-cli-driver/jumpstarter_cli_driver/__init__.py(1 hunks)packages/jumpstarter-cli-driver/jumpstarter_cli_driver/driver.py(1 hunks)packages/jumpstarter-cli-driver/jumpstarter_cli_driver/driver_test.py(1 hunks)packages/jumpstarter-cli-driver/pyproject.toml(1 hunks)packages/jumpstarter-cli/jumpstarter_cli/__init__.py(2 hunks)packages/jumpstarter-cli/pyproject.toml(1 hunks)packages/jumpstarter-driver-can/pyproject.toml(1 hunks)packages/jumpstarter-driver-composite/pyproject.toml(1 hunks)packages/jumpstarter-driver-dutlink/pyproject.toml(1 hunks)packages/jumpstarter-driver-network/pyproject.toml(1 hunks)packages/jumpstarter-driver-opendal/pyproject.toml(1 hunks)packages/jumpstarter-driver-power/pyproject.toml(1 hunks)packages/jumpstarter-driver-raspberrypi/pyproject.toml(1 hunks)packages/jumpstarter-driver-sdwire/pyproject.toml(1 hunks)pyproject.toml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/jumpstarter-cli-driver/README.md
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: e2e
- GitHub Check: build-and-push-image (jumpstarter-dev/jumpstarter-devspace .devfile/Containerfile.client)
- GitHub Check: build-and-push-image (jumpstarter-dev/jumpstarter-dev .devfile/Containerfile)
🔇 Additional comments (19)
packages/jumpstarter-cli-driver/jumpstarter_cli_driver/driver_test.py (1)
18-20: LGTM!The fixture correctly configures the anyio backend to use asyncio.
packages/jumpstarter-cli/jumpstarter_cli/__init__.py (2)
5-5: LGTM!The import follows the established pattern and is correctly placed with other similar imports.
15-15: LGTM!The command registration follows the established pattern and is correctly placed with other similar registrations.
packages/jumpstarter-driver-sdwire/pyproject.toml (1)
18-20: Ensure Consistency in the SDWire Entry Point.
The new entry pointSDWire = "jumpstarter_driver_sdwire.driver:SDWire"is added correctly. Please verify that the module path and class name exactly match the implementation and that the naming convention is consistent with other driver entry points within the project.packages/jumpstarter-driver-opendal/pyproject.toml (1)
18-20: Validate the New Opendal Adapter Entry Point.
The entry pointOpendal = "jumpstarter_driver_opendal.adapters:OpendalAdapter"is properly defined under[project.entry-points."jumpstarter.adapters"]. Confirm that this mapping correctly reflects the module structure and that it adheres to the project’s naming conventions used for adapters.packages/jumpstarter-driver-composite/pyproject.toml (1)
17-19: Confirm the Composite Driver Entry Point Integration.
The addition of theCompositeentry point with the mappingComposite = "jumpstarter_driver_composite.driver:Composite"is straightforward and appears correct. Ensure the module and class names are accurate and that this new driver aligns with the overall entry point naming scheme applied in the project.packages/jumpstarter-driver-power/pyproject.toml (1)
18-20: Review the MockPower Driver Entry Point.
The new entry pointMockPower = "jumpstarter_driver_power.driver:MockPower"under the[project.entry-points."jumpstarter.drivers"]section is well defined. Please double-check that the naming style is consistent with similar drivers in the ecosystem and that the referenced class exists in the specified module.packages/jumpstarter-driver-raspberrypi/pyproject.toml (1)
18-21: Confirm the Dual Entry Points for Raspberry Pi Drivers.
The new entry pointsDigitalInputandDigitalOutputmapping tojumpstarter_driver_raspberrypi.driver:DigitalInputandjumpstarter_driver_raspberrypi.driver:DigitalOutputare added correctly. This dual entry point setup provides a clear separation of functionality. It would be beneficial to ensure accompanying documentation (or in-code comments) clearly explains their usage to avoid any potential confusion regarding import semantics.packages/jumpstarter-driver-can/pyproject.toml (1)
19-23: Entry Points for CAN Drivers AddedThe new entry points under
[project.entry-points."jumpstarter.drivers"]forCan,IsoTpPython, andIsoTpSocketare formatted correctly. Please ensure that the corresponding driver classes withinjumpstarter_driver_can.driverexist and conform to the naming conventions expected by the CLI listing mechanism.packages/jumpstarter-cli/pyproject.toml (1)
16-16: New CLI Driver Dependency AddedThe addition of
"jumpstarter-cli-driver"in the dependencies list integrates the new driver CLI functionality into the main CLI. Verify that this dependency harmonizes with existing components and does not introduce any ambiguous import paths.packages/jumpstarter-cli-driver/pyproject.toml (7)
1-12: New Project Metadata ConfigurationThe metadata—including
name,dynamic,readme,license, andrequires-python—is correctly defined for this new package. Consider adding a brief project description later to better clarify the package’s purpose.
13-16: Dependencies SpecificationThe dependencies list (with
"jumpstarter-cli-common"and"asyncclick>=8.1.7.2") is appropriate for supporting asynchronous CLI operations.
18-25: Development Dependency Group DefinedThe
dependency-groupssection for dev dependencies is well established, ensuring robust testing support.
29-31: Wheel Packaging ConfigurationThe
wheelbuild target configuration correctly includes the packagejumpstarter_cli_driver, ensuring proper distribution.
32-35: VCS URLs for MetadataThe URLs for
Homepageandsource_archiveare appropriately set, facilitating traceability to the source repository.
36-39: Version Control IntegrationSourcing the version from the VCS with the specified raw options is an effective approach. Make sure the relative path (
'../../') is accurate with the repository structure.
40-42: Build System ConfigurationThe build system requirements using
hatchlingandhatch-vcsare correctly specified.packages/jumpstarter-driver-network/pyproject.toml (2)
19-24: Network Driver Entry Points ConfiguredThe entry points for network drivers (
TcpNetwork,UdpNetwork,UnixNetwork, andEchoNetwork) under the"jumpstarter.drivers"group are correctly declared. Ensure that each mapped class injumpstarter_driver_network.driverexists and follows the project’s naming conventions.
25-30: Adapter Entry Points for Network AdaptersThe adapter entry points (
TcpPortforward,UnixPortforward,Pexpect, andNovnc) under the"jumpstarter.adapters"group are properly defined. Confirm that the target classes injumpstarter_driver_network.adaptersare imported correctly and clear in their purpose.
packages/jumpstarter-cli-driver/jumpstarter_cli_driver/driver.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/jumpstarter-cli-driver/jumpstarter_cli_driver/driver.py (2)
1-5: Add docstring to clarify import semantics.Given the feedback about confusing import semantics, consider adding a module docstring explaining the purpose of each import:
+""" +Driver listing functionality for the Jumpstarter CLI. + +Imports: +- entry_points: Used to discover installed Jumpstarter drivers +- asyncclick: Async variant of Click for CLI command handling +- make_table: Utility for formatting driver information as a table +""" from importlib.metadata import entry_points import asyncclick as click from jumpstarter_cli_common import make_table
13-13: Consider more robust value formatting.The current string replacement might be fragile if the value contains multiple colons.
- click.echo(make_table(["NAME", "TYPE"], [{"NAME": e.name, "TYPE": e.value.replace(":", ".")} for e in drivers])) + click.echo(make_table(["NAME", "TYPE"], [{"NAME": e.name, "TYPE": e.value.split(":", 1)[0]} for e in drivers]))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/jumpstarter-cli-driver/jumpstarter_cli_driver/driver.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
packages/jumpstarter-cli-driver/jumpstarter_cli_driver/driver.py (1)
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#258
File: packages/jumpstarter-cli-driver/jumpstarter_cli_driver/driver.py:9-13
Timestamp: 2025-02-10T18:05:02.169Z
Learning: The `importlib.metadata.entry_points()` function is designed to never raise exceptions and safely returns an empty iterator when no entry points are found, making explicit error handling unnecessary.
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: pytest-matrix (3.13)
- GitHub Check: pytest-matrix (3.12)
- GitHub Check: pytest-matrix (3.11)
- GitHub Check: e2e
- GitHub Check: build-and-push-image (jumpstarter-dev/jumpstarter-devspace .devfile/Containerfile.client)
- GitHub Check: build-and-push-image (jumpstarter-dev/jumpstarter-dev .devfile/Containerfile)
- GitHub Check: build-and-push-image (jumpstarter-dev/jumpstarter Dockerfile)
🔇 Additional comments (1)
packages/jumpstarter-cli-driver/jumpstarter_cli_driver/driver.py (1)
7-12: LGTM! Clean implementation of driver discovery.The command implementation is well-structured with good empty state handling. The use of
entry_points()for driver discovery is appropriate and safe, as it never raises exceptions.
Summary by CodeRabbit
New Features
Documentation