feat(driver-vnc): create vnc driver #775
Conversation
✅ Deploy Preview for jumpstarter-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughAdds a new jumpstarter VNC driver package (code, tests, examples, README, pyproject), registers it in the workspace and docs, and extends the noVNC adapter to accept an encrypt flag and build connection URLs from parameters. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant VNClient
participant TCPChild as "TCP Child Driver"
participant NovncAdapter
participant Browser
User->>VNClient: session(encrypt?)
VNClient->>TCPChild: stream/connect -> get host:port
TCPChild-->>VNClient: host:port
VNClient->>NovncAdapter: start (client=TCPChild, encrypt=...)
NovncAdapter-->>VNClient: noVNC URL
VNClient-->>User: yield URL
alt open in browser
VNClient->>Browser: open URL
Browser-->>User: render viewer
end
User-->>VNClient: interrupt/exit
VNClient->>NovncAdapter: stop / cleanup
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
Outdated
Show resolved
Hide resolved
![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 (3)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py (1)
1-16: Composite behavior test is good; consider adding a negative caseThis test nicely verifies that
Vncproduces aCompositeClientwith atcpchild when correctly configured. You might optionally add a second test asserting that constructingVncwithout a"tcp"child raisesConfigurationError, to lock in the configuration contract enforced in__post_init__.packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.py (1)
1-3: RedundantVNClient = VNClientassignment
from .client import VNClientalready exposesVNClientat the package level, so the extraVNClient = VNClientline is redundant and can be dropped for clarity:-from .client import VNClient - -VNClient = VNClient +from .client import VNClientpackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (1)
33-57: CLI wiring is minimal but overrides composite children; consider whether child CLIs (e.g. tcp) should be exposed
VNClient.clibuilds a freshvncclick group and registers only thesessionsubcommand. UnlikeCompositeClient.cli, it does not iterate overself.childrenand expose their CLI commands (e.g.tcp, if present).If the VNC driver is meant to be a thin composite wrapper around a
tcpchild, this is fine. If you still want access to the underlying child driver CLIs (for debugging / power users), you might want to:
- Either delegate to
CompositeClient.cliand addsessionas an extra command, or- Manually attach selected child CLIs to
vnc.For example (conceptual pattern):
base = super().cli() # from CompositeClient @base.command() @click.option("--no-browser", is_flag=True, ...) def session(no_browser: bool): ... return baseNot strictly required for correctness, but worth deciding explicitly for UX consistency with other composite drivers.
📜 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 (9)
docs/source/reference/package-apis/drivers/vnc.md(1 hunks)packages/jumpstarter-driver-vnc/README.md(1 hunks)packages/jumpstarter-driver-vnc/examples/exporter.yaml(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py(1 hunks)packages/jumpstarter-driver-vnc/pyproject.toml(1 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
pyproject.toml: Top-levelpyproject.tomlshould define the UV workspace and include all packages
All packages inpackages/*andexamples/*directories must be included as workspace members in the top-levelpyproject.toml
Files:
pyproject.toml
packages/jumpstarter-driver-*/jumpstarter_driver_*/driver.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver class names should be in CamelCase and be descriptive with appropriate suffixes based on functionality: Power drivers should end with
*Power, Network drivers with*Network, Flasher drivers with*Flasher, Console drivers with*Console, Server drivers with*Server
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver implementations should follow existing code style validated with
make lint(fix withmake lint-fix), perform static type checking withmake ty-pkg-${package_name}, add comprehensive tests, and verify all tests pass withmake test-pkg-${package_name}ormake test
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Ruff should be used for code formatting and linting, excluding
jumpstarter-protocolpackage
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py
packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use
CompositeClientfromjumpstarter_driver_composite.clientfor composite drivers with child drivers.
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
packages/jumpstarter-driver-*/pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver package names should be lowercase with hyphens for multi-word names (e.g.,
my-driver,custom-power,device-controller)
packages/jumpstarter-driver-*/pyproject.toml: Driver packages must follow the naming patternjumpstarter-driver-<name>
Driver packages must register via thejumpstarter.driversentry point inpyproject.toml
Driver packages must depend onjumpstarterand specific hardware libraries in theirpyproject.toml
Files:
packages/jumpstarter-driver-vnc/pyproject.toml
packages/*/pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Each package's
pyproject.tomlmust include project metadata with Apache-2.0 license only
Files:
packages/jumpstarter-driver-vnc/pyproject.toml
🧠 Learnings (20)
📓 Common learnings
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must register via the `jumpstarter.drivers` entry point in `pyproject.toml`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must follow the naming pattern `jumpstarter-driver-<name>`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must depend on `jumpstarter` and specific hardware libraries in their `pyproject.toml`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py : Driver implementations should follow existing code style validated with `make lint` (fix with `make lint-fix`), perform static type checking with `make ty-pkg-${package_name}`, add comprehensive tests, and verify all tests pass with `make test-pkg-${package_name}` or `make test`
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must register via the `jumpstarter.drivers` entry point in `pyproject.toml`
Applied to files:
pyproject.tomldocs/source/reference/package-apis/drivers/vnc.mdpackages/jumpstarter-driver-vnc/pyproject.tomlpackages/jumpstarter-driver-vnc/README.md
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to pyproject.toml : Top-level `pyproject.toml` should define the UV workspace and include all packages
Applied to files:
pyproject.tomlpackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must follow the naming pattern `jumpstarter-driver-<name>`
Applied to files:
pyproject.tomldocs/source/reference/package-apis/drivers/vnc.mdpackages/jumpstarter-driver-vnc/pyproject.tomlpackages/jumpstarter-driver-vnc/README.md
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must depend on `jumpstarter` and specific hardware libraries in their `pyproject.toml`
Applied to files:
pyproject.tomldocs/source/reference/package-apis/drivers/vnc.mdpackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-cli-*/pyproject.toml : CLI packages must depend on `jumpstarter` and `jumpstarter-cli-common` in their `pyproject.toml`
Applied to files:
pyproject.tomlpackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Core packages must depend on `jumpstarter-protocol`
Applied to files:
pyproject.toml
📚 Learning: 2025-09-06T05:25:18.184Z
Learnt from: michalskrivanek
Repo: jumpstarter-dev/jumpstarter PR: 608
File: packages/jumpstarter-cli/jumpstarter_cli/run.py:0-0
Timestamp: 2025-09-06T05:25:18.184Z
Learning: Zombie process reaping in jumpstarter is useful beyond just PID 1 cases, as drivers can leave behind processes in various execution contexts.
Applied to files:
pyproject.toml
📚 Learning: 2025-09-06T05:25:18.184Z
Learnt from: michalskrivanek
Repo: jumpstarter-dev/jumpstarter PR: 608
File: packages/jumpstarter-cli/jumpstarter_cli/run.py:0-0
Timestamp: 2025-09-06T05:25:18.184Z
Learning: The Shell driver in jumpstarter can spawn multiple processes and leave them behind, creating zombie processes that need to be reaped even in non-PID 1 scenarios.
Applied to files:
pyproject.toml
📚 Learning: 2025-10-05T07:18:21.352Z
Learnt from: bennyz
Repo: jumpstarter-dev/jumpstarter PR: 661
File: packages/jumpstarter-driver-iscsi/jumpstarter_driver_iscsi/driver.py:391-397
Timestamp: 2025-10-05T07:18:21.352Z
Learning: The iSCSI driver in `packages/jumpstarter-driver-iscsi/jumpstarter_driver_iscsi/driver.py` is designed to support only one LUN at a time, so hardcoding LUN ID 0 in the `add_lun` method is acceptable.
Applied to files:
pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.pydocs/source/reference/package-apis/drivers/vnc.mdpackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.pypackages/jumpstarter-driver-vnc/pyproject.tomlpackages/jumpstarter-driver-vnc/README.md
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.pydocs/source/reference/package-apis/drivers/vnc.mdpackages/jumpstarter-driver-vnc/pyproject.tomlpackages/jumpstarter-driver-vnc/README.md
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py : Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use `CompositeClient` from `jumpstarter_driver_composite.client` for composite drivers with child drivers.
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.pypackages/jumpstarter-driver-vnc/README.md
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: All driver packages should have a symlink in `docs/source/reference/package-apis/drivers/` pointing to the driver's README.md
Applied to files:
docs/source/reference/package-apis/drivers/vnc.md
📚 Learning: 2025-01-29T11:52:43.554Z
Learnt from: bennyz
Repo: jumpstarter-dev/jumpstarter PR: 241
File: packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py:52-60
Timestamp: 2025-01-29T11:52:43.554Z
Learning: The TFTP driver (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/driver.py) handles all low-level concerns like path validation, error handling, and checksum computation. The client (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py) should remain simple as it delegates these responsibilities to the driver.
Applied to files:
docs/source/reference/package-apis/drivers/vnc.mdpackages/jumpstarter-driver-vnc/README.md
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py : Driver implementations should follow existing code style validated with `make lint` (fix with `make lint-fix`), perform static type checking with `make ty-pkg-${package_name}`, add comprehensive tests, and verify all tests pass with `make test-pkg-${package_name}` or `make test`
Applied to files:
docs/source/reference/package-apis/drivers/vnc.mdpackages/jumpstarter-driver-vnc/pyproject.tomlpackages/jumpstarter-driver-vnc/README.md
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-composite/pyproject.toml : Composite drivers that have child drivers should inherit from `CompositeClient` in `jumpstarter_driver_composite.client` and have a dependency on `jumpstarter-driver-composite` in `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.pypackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver package names should be lowercase with hyphens for multi-word names (e.g., `my-driver`, `custom-power`, `device-controller`)
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to examples/*/pyproject.toml : Example packages should depend on relevant driver packages in their `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/*/pyproject.toml : Each package's `pyproject.toml` must include project metadata with Apache-2.0 license only
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
🧬 Code graph analysis (2)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (4)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (1)
client(19-20)packages/jumpstarter-driver-composite/jumpstarter_driver_composite/client.py (1)
CompositeClient(28-55)packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py (1)
NovncAdapter(13-31)packages/jumpstarter/jumpstarter/client/decorators.py (1)
driver_click_group(13-54)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.py (2)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (1)
client(19-20)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (1)
VNClient(12-57)
⏰ 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). (11)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: build
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: e2e
- GitHub Check: Pages changed - jumpstarter-docs
🔇 Additional comments (6)
pyproject.toml (1)
38-38: Workspace registration for VNC driver looks correctAdding
jumpstarter-driver-vnc = { workspace = true }keeps the workspace list in sync with the new package.packages/jumpstarter-driver-vnc/examples/exporter.yaml (1)
1-17: Example exporter config is consistent with the VNC driverThe
typepaths andchildren.tcpnaming line up withVnc’s TCP child requirement and the README example; config looks ready to use as a template.docs/source/reference/package-apis/drivers/vnc.md (1)
1-1: Driver docs symlink/reference looks consistentThe reference to
packages/jumpstarter-driver-vnc/README.mdfollows the established pattern for driver API docs.packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (1)
1-20: VNC driver implementation and TCP child validation look solidThe
Vncdriver correctly enforces the presence of a"tcp"child and exposes the VNClient path viaclient(), which aligns with the composite pattern and example configs. The__post_init__delegation to the base class is also handled safely.packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (2)
11-13: Verify@dataclassdecorator compatibility withCompositeClientinitialization
VNClientuses@dataclass(kw_only=True)but declares no additional fields. In Python,@dataclassgenerates an__init__method that may override or conflict with the parent class's__init__. This could interfere withCompositeClient's initialization (e.g., setting upchildren,tcpattributes) if the parent expects a custom initialization flow.If
CompositeClientalready handles all necessary field initialization andVNClientadds no new fields, removing the dataclass decorator would be simpler and safer:-@dataclass(kw_only=True) -class VNClient(CompositeClient): +class VNClient(CompositeClient):Compare this pattern with how other composite drivers in the codebase subclass
CompositeClientto ensure consistency.
15-31: Unable to complete verification. The repository is not accessible via shell, and theNovncAdapterimplementation is not publicly documented or available through web search. While the web search confirms that Jumpstarter generally uses async context managers (@asynccontextmanager) and follows the context-manager pattern for adapters, I cannot verify the specific implementation ofNovncAdapter, whether the current code inclient.pyactually fails at runtime, or whether the proposed solution aligns with project conventions.To properly address this review comment, I would need:
- Access to the actual
NovncAdapterdefinition injumpstarter_driver_network.adapters.novnc- Confirmation of whether
session()is intended to be synchronous or asynchronous- Understanding of how
TemporaryTcpListenerlifecycle is managed- Visibility into how other similar adapters in the project are consumed
aesteve-rh
force-pushed
the
vnc-driver
branch
from
2ca99eb to
7d99539
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 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 (9)
docs/source/reference/package-apis/drivers/vnc.md(1 hunks)packages/jumpstarter-driver-vnc/README.md(1 hunks)packages/jumpstarter-driver-vnc/examples/exporter.yaml(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py(1 hunks)packages/jumpstarter-driver-vnc/pyproject.toml(1 hunks)pyproject.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/init.py
- packages/jumpstarter-driver-vnc/README.md
- pyproject.toml
- docs/source/reference/package-apis/drivers/vnc.md
- packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py
- packages/jumpstarter-driver-vnc/examples/exporter.yaml
🧰 Additional context used
📓 Path-based instructions (6)
packages/jumpstarter-driver-*/pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver package names should be lowercase with hyphens for multi-word names (e.g.,
my-driver,custom-power,device-controller)
packages/jumpstarter-driver-*/pyproject.toml: Driver packages must follow the naming patternjumpstarter-driver-<name>
Driver packages must register via thejumpstarter.driversentry point inpyproject.toml
Driver packages must depend onjumpstarterand specific hardware libraries in theirpyproject.toml
Files:
packages/jumpstarter-driver-vnc/pyproject.toml
packages/*/pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Each package's
pyproject.tomlmust include project metadata with Apache-2.0 license only
Files:
packages/jumpstarter-driver-vnc/pyproject.toml
packages/jumpstarter-driver-*/jumpstarter_driver_*/driver.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver class names should be in CamelCase and be descriptive with appropriate suffixes based on functionality: Power drivers should end with
*Power, Network drivers with*Network, Flasher drivers with*Flasher, Console drivers with*Console, Server drivers with*Server
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver implementations should follow existing code style validated with
make lint(fix withmake lint-fix), perform static type checking withmake ty-pkg-${package_name}, add comprehensive tests, and verify all tests pass withmake test-pkg-${package_name}ormake test
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Ruff should be used for code formatting and linting, excluding
jumpstarter-protocolpackage
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use
CompositeClientfromjumpstarter_driver_composite.clientfor composite drivers with child drivers.
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must register via the `jumpstarter.drivers` entry point in `pyproject.toml`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must follow the naming pattern `jumpstarter-driver-<name>`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must depend on `jumpstarter` and specific hardware libraries in their `pyproject.toml`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py : Driver implementations should follow existing code style validated with `make lint` (fix with `make lint-fix`), perform static type checking with `make ty-pkg-${package_name}`, add comprehensive tests, and verify all tests pass with `make test-pkg-${package_name}` or `make test`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Core packages must depend on `jumpstarter-protocol`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver package names should be lowercase with hyphens for multi-word names (e.g., `my-driver`, `custom-power`, `device-controller`)
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-cli-*/pyproject.toml : CLI packages must depend on `jumpstarter` and `jumpstarter-cli-common` in their `pyproject.toml`
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must register via the `jumpstarter.drivers` entry point in `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must follow the naming pattern `jumpstarter-driver-<name>`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must depend on `jumpstarter` and specific hardware libraries in their `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-cli-*/pyproject.toml : CLI packages must depend on `jumpstarter` and `jumpstarter-cli-common` in their `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver package names should be lowercase with hyphens for multi-word names (e.g., `my-driver`, `custom-power`, `device-controller`)
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to examples/*/pyproject.toml : Example packages should depend on relevant driver packages in their `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.tomlpackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py : Driver implementations should follow existing code style validated with `make lint` (fix with `make lint-fix`), perform static type checking with `make ty-pkg-${package_name}`, add comprehensive tests, and verify all tests pass with `make test-pkg-${package_name}` or `make test`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to pyproject.toml : Top-level `pyproject.toml` should define the UV workspace and include all packages
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/*/pyproject.toml : Each package's `pyproject.toml` must include project metadata with Apache-2.0 license only
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.tomlpackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/driver.py : Driver class names should be in CamelCase and be descriptive with appropriate suffixes based on functionality: Power drivers should end with `*Power`, Network drivers with `*Network`, Flasher drivers with `*Flasher`, Console drivers with `*Console`, Server drivers with `*Server`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-composite/pyproject.toml : Composite drivers that have child drivers should inherit from `CompositeClient` in `jumpstarter_driver_composite.client` and have a dependency on `jumpstarter-driver-composite` in `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py : Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use `CompositeClient` from `jumpstarter_driver_composite.client` for composite drivers with child drivers.
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
🪛 GitHub Actions: Spell Check
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
[warning] 11-11: Initialise should be Initialize.
[error] 11-11: Initialise should be Initialize
🪛 GitHub Check: typos
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
[warning] 11-11:
"Initialise" should be "Initialize".
⏰ 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). (10)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: e2e
🔇 Additional comments (1)
packages/jumpstarter-driver-vnc/pyproject.toml (1)
1-48: Driver package metadata and entry-point wiring look good; double-check test discovery
- Package name, license, and dependencies match the driver-package guidelines (lowercase hyphenated name, Apache-2.0, depends on
jumpstarterplus composite/network drivers).- The
jumpstarter.driversentry point correctly exposesjumpstarter_driver_vnc.driver:Vnc, so the driver should be discoverable like the others.- One thing to verify:
testpaths = ["jumpstarter_driver_vnc"]will limit pytest to that package; if yourdriver_test.pylives at the project root (packages/jumpstarter-driver-vnc/driver_test.py), either move it underjumpstarter_driver_vnc/or broadentestpaths(e.g.["."]) so the tests run as expected.Based on learnings, this aligns with the standard driver packaging rules.
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
Outdated
Show resolved
Hide resolved
aesteve-rh
force-pushed
the
vnc-driver
branch
2 times, most recently
from
c9bb3c2 to
0b403fe
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (2)
33-40: Consider reusingCompositeClient.cli()to expose child CLIs and the log‑level optionBy defining a fresh
vncgroup here and not callingsuper().cli(), this client loses the built‑in--log-leveloption and any CLI commands exposed by child drivers (like the underlyingtcpclient).If you want the standard composite behavior plus this
sessioncommand, consider:def cli(self) -> click.Command: base = super().cli() @base.command() @click.option( "--browser/--no-browser", default=True, help="Open the session in a web browser.", ) def session(browser: bool) -> None: ... return baseThis keeps VNClient’s custom UX while still following the composite CLI pattern.
40-56: Simplify waiting for Ctrl+C and drop dependency onportal.call(asyncio.Event().wait)In this synchronous Click command, the only goal after opening the session is to keep it alive until the user presses Ctrl+C. Using
self.portal.call(asyncio.Event().wait)couples the CLI to AnyIO/asyncio cancellation semantics and adds complexity without much benefit.Once
session()bridgesNovncAdaptervia the portal (as suggested above), you can make the CLI logic purely synchronous:- @vnc.command() - @click.option("--browser/--no-browser", default=True, help="Open the session in a web browser.") - def session(browser: bool): - """Open a VNC session.""" - # The NovncAdapter is a blocking context manager that runs in a thread. - # We can enter it, open the browser, and then just wait for the user - # to press Ctrl+C to exit. The adapter handles the background work. - with self.session() as url: - click.echo(f"To connect, please visit: {url}") - if browser: - webbrowser.open(url) - click.echo("Press Ctrl+C to close the VNC session.") - try: - # Use the client's own portal to wait for cancellation. - self.portal.call(asyncio.Event().wait) - except (KeyboardInterrupt, anyio.get_cancelled_exc_class()): - click.echo("\nClosing VNC session.") + @vnc.command() + @click.option( + "--browser/--no-browser", + default=True, + help="Open the session in a web browser.", + ) + def session(browser: bool) -> None: + """Open a VNC session.""" + with self.session() as url: + click.echo(f"To connect, please visit: {url}") + if browser: + webbrowser.open(url) + click.echo("Press Ctrl+C to close the VNC session.") + try: + while True: + time.sleep(1) + except KeyboardInterrupt: + click.echo("\nClosing VNC session.")With this change you can also simplify the imports at the top:
-from __future__ import annotations - -import asyncio -import contextlib -import typing -import webbrowser - -import anyio -import click +from __future__ import annotations + +import contextlib +import time +import typing +import webbrowser + +import clickThis keeps the VNC session open exactly as long as the context manager is active, and makes the control flow and signal handling for the CLI much easier to reason about.
Please verify this behavior end‑to‑end (including Ctrl+C handling) under your actual
serve()/portal setup.packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py (2)
17-30: Import the realVncdriver instead of redefining it in testsThis local
Vncclass mirrors the productionjumpstarter_driver_vnc.driver.Vncimplementation (same__post_init__check andclient()path). Keeping a duplicate here risks tests going green even if the real driver changes and violates these expectations.You can simplify and make the tests more robust by importing the real driver:
-from jumpstarter.driver import Driver - - -class FakeTcpDriver(Driver): - @classmethod - def client(cls) -> str: - return "jumpstarter.client.DriverClient" - - -class Vnc(Driver): - """A mock VNC driver for testing.""" - - def __init__(self, **kwargs): - super().__init__(**kwargs) - - def __post_init__(self): - super().__post_init__() - if "tcp" not in self.children: - raise ConfigurationError("A tcp child is required for Vnc") - - @classmethod - def client(cls) -> str: - return "jumpstarter_driver_vnc.client.VNClient" +from jumpstarter.driver import Driver +from jumpstarter_driver_vnc.driver import Vnc + + +class FakeTcpDriver(Driver): + @classmethod + def client(cls) -> str: + return "jumpstarter.client.DriverClient"This way, if the real
Vncdriver’s__post_init__orclient()implementation ever changes unexpectedly, these tests will catch it.
33-47: Tests exercise the right invariants for the VNC driverThese tests correctly validate that:
- the served client for a Vnc with a
tcpchild is aCompositeClientand exposes atcpattribute, and- constructing Vnc without a
tcpchild raises the expectedConfigurationErrorwith the right message.Optionally, you could tighten the first test by asserting something about
client.tcp’s type (e.g., that it’s aDriverClient/FakeTcpDriverinstance), but the current checks already cover the key behavior.
📜 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 (9)
docs/source/reference/package-apis/drivers/vnc.md(1 hunks)packages/jumpstarter-driver-vnc/README.md(1 hunks)packages/jumpstarter-driver-vnc/examples/exporter.yaml(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py(1 hunks)packages/jumpstarter-driver-vnc/pyproject.toml(1 hunks)pyproject.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- pyproject.toml
- docs/source/reference/package-apis/drivers/vnc.md
- packages/jumpstarter-driver-vnc/README.md
- packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/init.py
- packages/jumpstarter-driver-vnc/pyproject.toml
- packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
- packages/jumpstarter-driver-vnc/examples/exporter.yaml
🧰 Additional context used
📓 Path-based instructions (3)
packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver implementations should follow existing code style validated with
make lint(fix withmake lint-fix), perform static type checking withmake ty-pkg-${package_name}, add comprehensive tests, and verify all tests pass withmake test-pkg-${package_name}ormake test
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Ruff should be used for code formatting and linting, excluding
jumpstarter-protocolpackage
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use
CompositeClientfromjumpstarter_driver_composite.clientfor composite drivers with child drivers.
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must register via the `jumpstarter.drivers` entry point in `pyproject.toml`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must follow the naming pattern `jumpstarter-driver-<name>`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must depend on `jumpstarter` and specific hardware libraries in their `pyproject.toml`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py : Driver implementations should follow existing code style validated with `make lint` (fix with `make lint-fix`), perform static type checking with `make ty-pkg-${package_name}`, add comprehensive tests, and verify all tests pass with `make test-pkg-${package_name}` or `make test`
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py : Driver implementations should follow existing code style validated with `make lint` (fix with `make lint-fix`), perform static type checking with `make ty-pkg-${package_name}`, add comprehensive tests, and verify all tests pass with `make test-pkg-${package_name}` or `make test`
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-composite/pyproject.toml : Composite drivers that have child drivers should inherit from `CompositeClient` in `jumpstarter_driver_composite.client` and have a dependency on `jumpstarter-driver-composite` in `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py : Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use `CompositeClient` from `jumpstarter_driver_composite.client` for composite drivers with child drivers.
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
🧬 Code graph analysis (2)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py (3)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (2)
client(17-19)Vnc(7-19)packages/jumpstarter-driver-composite/jumpstarter_driver_composite/client.py (1)
CompositeClient(28-55)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (1)
tcp(23-25)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (4)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (1)
client(17-19)packages/jumpstarter-driver-composite/jumpstarter_driver_composite/client.py (1)
CompositeClient(28-55)packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py (1)
NovncAdapter(13-31)packages/jumpstarter/jumpstarter/client/decorators.py (1)
driver_click_group(13-54)
⏰ 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). (11)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: build
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: e2e
🔇 Additional comments (2)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (2)
19-25: VNClient composition andtcpaccessor look correctExtending
CompositeClientand exposing the"tcp"child via thetcpproperty matches the driver contract and the tests indriver_test.py; no changes needed here.Based on learnings, this aligns with the composite-driver pattern for client implementations.
27-31: Verify NovncAdapter usage pattern in client.py against actual implementationThe proposed fix using
self.portal.wrap_async_context_manager()requires manual verification of three things:
- Confirm
NovncAdapteris truly an async context manager by checkingjumpstarter_driver_network/adapters/novnc.py- Verify the client has a
portalattribute of typeBlockingPortal- Confirm
BlockingPortal.wrap_async_context_manager()is available in your AnyIO version and used correctlyReview the existing
TcpPortforwardAdapterusage in the codebase as it likely demonstrates a similar async/sync bridging pattern.
aesteve-rh
force-pushed
the
vnc-driver
branch
from
0b403fe to
4e02b37
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.py (1)
1-3: Remove redundantVNClientself-alias
VNClient = VNClientis a no-op and can safely be dropped; the import already exposesVNClientat the package level.-from .client import VNClient - -VNClient = VNClient +from .client import VNClientThis keeps the public API the same while avoiding a confusing self-assignment.
📜 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 (10)
docs/source/reference/package-apis/drivers/index.md(2 hunks)docs/source/reference/package-apis/drivers/vnc.md(1 hunks)packages/jumpstarter-driver-vnc/README.md(1 hunks)packages/jumpstarter-driver-vnc/examples/exporter.yaml(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py(1 hunks)packages/jumpstarter-driver-vnc/pyproject.toml(1 hunks)pyproject.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- docs/source/reference/package-apis/drivers/index.md
- packages/jumpstarter-driver-vnc/README.md
- packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py
- pyproject.toml
- packages/jumpstarter-driver-vnc/examples/exporter.yaml
- packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
🧰 Additional context used
📓 Path-based instructions (5)
packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver implementations should follow existing code style validated with
make lint(fix withmake lint-fix), perform static type checking withmake ty-pkg-${package_name}, add comprehensive tests, and verify all tests pass withmake test-pkg-${package_name}ormake test
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Ruff should be used for code formatting and linting, excluding
jumpstarter-protocolpackage
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
packages/jumpstarter-driver-*/pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver package names should be lowercase with hyphens for multi-word names (e.g.,
my-driver,custom-power,device-controller)
packages/jumpstarter-driver-*/pyproject.toml: Driver packages must follow the naming patternjumpstarter-driver-<name>
Driver packages must register via thejumpstarter.driversentry point inpyproject.toml
Driver packages must depend onjumpstarterand specific hardware libraries in theirpyproject.toml
Files:
packages/jumpstarter-driver-vnc/pyproject.toml
packages/*/pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Each package's
pyproject.tomlmust include project metadata with Apache-2.0 license only
Files:
packages/jumpstarter-driver-vnc/pyproject.toml
packages/jumpstarter-driver-*/jumpstarter_driver_*/driver.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver class names should be in CamelCase and be descriptive with appropriate suffixes based on functionality: Power drivers should end with
*Power, Network drivers with*Network, Flasher drivers with*Flasher, Console drivers with*Console, Server drivers with*Server
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must register via the `jumpstarter.drivers` entry point in `pyproject.toml`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must follow the naming pattern `jumpstarter-driver-<name>`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py : Driver implementations should follow existing code style validated with `make lint` (fix with `make lint-fix`), perform static type checking with `make ty-pkg-${package_name}`, add comprehensive tests, and verify all tests pass with `make test-pkg-${package_name}` or `make test`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must depend on `jumpstarter` and specific hardware libraries in their `pyproject.toml`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Core packages must depend on `jumpstarter-protocol`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver package names should be lowercase with hyphens for multi-word names (e.g., `my-driver`, `custom-power`, `device-controller`)
Learnt from: bennyz
Repo: jumpstarter-dev/jumpstarter PR: 241
File: packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py:52-60
Timestamp: 2025-01-29T11:52:43.554Z
Learning: The TFTP driver (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/driver.py) handles all low-level concerns like path validation, error handling, and checksum computation. The client (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py) should remain simple as it delegates these responsibilities to the driver.
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: All driver packages should have a symlink in `docs/source/reference/package-apis/drivers/` pointing to the driver's README.md
Applied to files:
docs/source/reference/package-apis/drivers/vnc.md
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Applied to files:
docs/source/reference/package-apis/drivers/vnc.mdpackages/jumpstarter-driver-vnc/pyproject.tomlpackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
Applied to files:
docs/source/reference/package-apis/drivers/vnc.mdpackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.pypackages/jumpstarter-driver-vnc/pyproject.tomlpackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must follow the naming pattern `jumpstarter-driver-<name>`
Applied to files:
docs/source/reference/package-apis/drivers/vnc.mdpackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must register via the `jumpstarter.drivers` entry point in `pyproject.toml`
Applied to files:
docs/source/reference/package-apis/drivers/vnc.mdpackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must depend on `jumpstarter` and specific hardware libraries in their `pyproject.toml`
Applied to files:
docs/source/reference/package-apis/drivers/vnc.mdpackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-01-29T11:52:43.554Z
Learnt from: bennyz
Repo: jumpstarter-dev/jumpstarter PR: 241
File: packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py:52-60
Timestamp: 2025-01-29T11:52:43.554Z
Learning: The TFTP driver (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/driver.py) handles all low-level concerns like path validation, error handling, and checksum computation. The client (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py) should remain simple as it delegates these responsibilities to the driver.
Applied to files:
docs/source/reference/package-apis/drivers/vnc.md
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py : Driver implementations should follow existing code style validated with `make lint` (fix with `make lint-fix`), perform static type checking with `make ty-pkg-${package_name}`, add comprehensive tests, and verify all tests pass with `make test-pkg-${package_name}` or `make test`
Applied to files:
docs/source/reference/package-apis/drivers/vnc.mdpackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-cli-*/pyproject.toml : CLI packages must depend on `jumpstarter` and `jumpstarter-cli-common` in their `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver package names should be lowercase with hyphens for multi-word names (e.g., `my-driver`, `custom-power`, `device-controller`)
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to examples/*/pyproject.toml : Example packages should depend on relevant driver packages in their `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to pyproject.toml : Top-level `pyproject.toml` should define the UV workspace and include all packages
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/*/pyproject.toml : Each package's `pyproject.toml` must include project metadata with Apache-2.0 license only
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/driver.py : Driver class names should be in CamelCase and be descriptive with appropriate suffixes based on functionality: Power drivers should end with `*Power`, Network drivers with `*Network`, Flasher drivers with `*Flasher`, Console drivers with `*Console`, Server drivers with `*Server`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-composite/pyproject.toml : Composite drivers that have child drivers should inherit from `CompositeClient` in `jumpstarter_driver_composite.client` and have a dependency on `jumpstarter-driver-composite` in `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py : Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use `CompositeClient` from `jumpstarter_driver_composite.client` for composite drivers with child drivers.
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
🧬 Code graph analysis (1)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (1)
packages/jumpstarter/jumpstarter/driver/base.py (1)
Driver(57-274)
⏰ 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). (11)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: build
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: e2e
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
🔇 Additional comments (2)
docs/source/reference/package-apis/drivers/vnc.md (1)
1-1: Docs symlink target matches driver README locationThe relative path correctly points from the drivers docs directory to
packages/jumpstarter-driver-vnc/README.md, aligning with the “docs symlink to driver README” convention.
Based on learnings, this matches the required docs structure.packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (1)
7-19: Vnc driver initialization and client mapping look correct
__post_init__correctly delegates to the baseDriverinitializer and enforces the presence of a"tcp"child, failing fast withConfigurationErrorwhen misconfigured. Theclient()classmethod cleanly returns the fully qualified VNClient path, matching the entry point and package layout.
As per coding guidelines, this satisfies the Driver base API and driver/client separation.
aesteve-rh
force-pushed
the
vnc-driver
branch
from
4e02b37 to
fd5c70e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/jumpstarter-driver-vnc/pyproject.toml (1)
28-28: Correct thesource_archiveURL to point to the main monorepo.The VCS URL should reference the main
jumpstarterrepository, not a standalone driver repo. The driver code is part of the monorepo atpackages/jumpstarter-driver-vnc/.Apply this diff:
-source_archive = "https://github.com/jumpstarter-dev/jumpstarter-driver-vnc/archive/{commit_hash}.zip" +source_archive = "https://github.com/jumpstarter-dev/jumpstarter/archive/{commit_hash}.zip"
🧹 Nitpick comments (1)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (1)
1-32: VNClient structure and NovncAdapter wrapper look solid (optional typing tweak)The client cleanly extends
CompositeClient, narrows thetcpchild via a dedicated property, and exposes a smallsession()context manager overNovncAdapter, which keeps the higher‑level API tidy.If
CompositeClient.childrenis typed as a generic mapping ofDriverClients, static type-checkers may complain about returning aTCPClientfromself.children["tcp"]. To keepty-pkg-jumpstarter-driver-vnchappy without changing runtime behavior, you could narrow the type explicitly:@@ @property def tcp(self) -> TCPClient: """Get the TCP client.""" - return self.children["tcp"] + return typing.cast("TCPClient", self.children["tcp"])As per coding guidelines, this helps keep static typing clean for new driver packages.
📜 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 (10)
docs/source/reference/package-apis/drivers/index.md(2 hunks)docs/source/reference/package-apis/drivers/vnc.md(1 hunks)packages/jumpstarter-driver-vnc/README.md(1 hunks)packages/jumpstarter-driver-vnc/examples/exporter.yaml(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py(1 hunks)packages/jumpstarter-driver-vnc/pyproject.toml(1 hunks)pyproject.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/jumpstarter-driver-vnc/README.md
- packages/jumpstarter-driver-vnc/examples/exporter.yaml
- docs/source/reference/package-apis/drivers/vnc.md
- pyproject.toml
- packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/init.py
- packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
- packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py
🧰 Additional context used
📓 Path-based instructions (5)
packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use
CompositeClientfromjumpstarter_driver_composite.clientfor composite drivers with child drivers.
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver implementations should follow existing code style validated with
make lint(fix withmake lint-fix), perform static type checking withmake ty-pkg-${package_name}, add comprehensive tests, and verify all tests pass withmake test-pkg-${package_name}ormake test
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Ruff should be used for code formatting and linting, excluding
jumpstarter-protocolpackage
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
packages/jumpstarter-driver-*/pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver package names should be lowercase with hyphens for multi-word names (e.g.,
my-driver,custom-power,device-controller)
packages/jumpstarter-driver-*/pyproject.toml: Driver packages must follow the naming patternjumpstarter-driver-<name>
Driver packages must register via thejumpstarter.driversentry point inpyproject.toml
Driver packages must depend onjumpstarterand specific hardware libraries in theirpyproject.toml
Files:
packages/jumpstarter-driver-vnc/pyproject.toml
packages/*/pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Each package's
pyproject.tomlmust include project metadata with Apache-2.0 license only
Files:
packages/jumpstarter-driver-vnc/pyproject.toml
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must register via the `jumpstarter.drivers` entry point in `pyproject.toml`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must follow the naming pattern `jumpstarter-driver-<name>`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py : Driver implementations should follow existing code style validated with `make lint` (fix with `make lint-fix`), perform static type checking with `make ty-pkg-${package_name}`, add comprehensive tests, and verify all tests pass with `make test-pkg-${package_name}` or `make test`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must depend on `jumpstarter` and specific hardware libraries in their `pyproject.toml`
Learnt from: bennyz
Repo: jumpstarter-dev/jumpstarter PR: 241
File: packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py:52-60
Timestamp: 2025-01-29T11:52:43.554Z
Learning: The TFTP driver (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/driver.py) handles all low-level concerns like path validation, error handling, and checksum computation. The client (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py) should remain simple as it delegates these responsibilities to the driver.
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Core packages must depend on `jumpstarter-protocol`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver package names should be lowercase with hyphens for multi-word names (e.g., `my-driver`, `custom-power`, `device-controller`)
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-cli-*/pyproject.toml : CLI packages must depend on `jumpstarter` and `jumpstarter-cli-common` in their `pyproject.toml`
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.pypackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py : Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use `CompositeClient` from `jumpstarter_driver_composite.client` for composite drivers with child drivers.
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: All driver packages should have a symlink in `docs/source/reference/package-apis/drivers/` pointing to the driver's README.md
Applied to files:
docs/source/reference/package-apis/drivers/index.md
📚 Learning: 2025-02-08T16:08:34.616Z
Learnt from: bennyz
Repo: jumpstarter-dev/jumpstarter PR: 267
File: packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/driver.py:32-40
Timestamp: 2025-02-08T16:08:34.616Z
Learning: In the TFTP driver (jumpstarter_driver_tftp), the TFTP server component is read-only, while file management operations (upload, delete) are handled through the driver's client interface, not through the TFTP protocol itself.
Applied to files:
docs/source/reference/package-apis/drivers/index.md
📚 Learning: 2025-01-29T11:52:43.554Z
Learnt from: bennyz
Repo: jumpstarter-dev/jumpstarter PR: 241
File: packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py:52-60
Timestamp: 2025-01-29T11:52:43.554Z
Learning: The TFTP driver (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/driver.py) handles all low-level concerns like path validation, error handling, and checksum computation. The client (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py) should remain simple as it delegates these responsibilities to the driver.
Applied to files:
docs/source/reference/package-apis/drivers/index.md
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Applied to files:
docs/source/reference/package-apis/drivers/index.mdpackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must register via the `jumpstarter.drivers` entry point in `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must follow the naming pattern `jumpstarter-driver-<name>`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must depend on `jumpstarter` and specific hardware libraries in their `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-cli-*/pyproject.toml : CLI packages must depend on `jumpstarter` and `jumpstarter-cli-common` in their `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver package names should be lowercase with hyphens for multi-word names (e.g., `my-driver`, `custom-power`, `device-controller`)
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to examples/*/pyproject.toml : Example packages should depend on relevant driver packages in their `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to pyproject.toml : Top-level `pyproject.toml` should define the UV workspace and include all packages
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/*/pyproject.toml : Each package's `pyproject.toml` must include project metadata with Apache-2.0 license only
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py : Driver implementations should follow existing code style validated with `make lint` (fix with `make lint-fix`), perform static type checking with `make ty-pkg-${package_name}`, add comprehensive tests, and verify all tests pass with `make test-pkg-${package_name}` or `make test`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/driver.py : Driver class names should be in CamelCase and be descriptive with appropriate suffixes based on functionality: Power drivers should end with `*Power`, Network drivers with `*Network`, Flasher drivers with `*Flasher`, Console drivers with `*Console`, Server drivers with `*Server`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-composite/pyproject.toml : Composite drivers that have child drivers should inherit from `CompositeClient` in `jumpstarter_driver_composite.client` and have a dependency on `jumpstarter-driver-composite` in `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
🧬 Code graph analysis (1)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (4)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (1)
client(17-19)packages/jumpstarter-driver-composite/jumpstarter_driver_composite/client.py (1)
CompositeClient(28-55)packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py (1)
NovncAdapter(13-31)packages/jumpstarter/jumpstarter/client/base.py (1)
call(42-52)
⏰ 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). (10)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: build
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: e2e
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
🔇 Additional comments (3)
packages/jumpstarter-driver-vnc/pyproject.toml (1)
1-48: Package metadata and driver registration look good otherwise.Project naming follows the
jumpstarter-driver-<name>pattern, Apache-2.0 license is set correctly, dependencies (jumpstarter, composite and network drivers) are appropriate, and the entry point is properly configured underjumpstarter.drivers.vnc. Based on learnings, the overall structure aligns with driver packaging guidelines.docs/source/reference/package-apis/drivers/index.md (1)
45-45: VNC driver docs entry and toctree wiring look consistentThe new VNC bullet under Communication Drivers and the
vnc.mdentry in the hidden toctree match existing patterns and thejumpstarter-driver-vncpackage name. Together with thevnc.md→ README linkage in this PR, this satisfies the “each driver has a docs symlink entry” guideline.Based on learnings, this matches the expected driver docs structure.
Also applies to: 115-115
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (1)
33-58: CLI design usingdriver_click_groupand portal-based wait achieves the intended UXThe custom
vncgroup andsessionsubcommand are straightforward and match the driver's goal: open a tunneled VNC session, optionally launch a browser, and keep the NovncAdapter context open until the user hits Ctrl+C. Using the client's existingportalinstead ofanyio.from_thread.run()avoids the MissingTokenError risk and integrates correctly with Jumpstarter's async runtime.Both
asyncio.Event().waitandanyio.sleep_foreverare functionally equivalent when called throughportal.call()for blocking until cancellation. If you prefer an AnyIO-only approach over mixing AsyncIO primitives,anyio.sleep_foreveris the idiomatic choice and avoids constructing an unusedasyncio.Event. The current implementation is correct and functional as-is.
aesteve-rh
force-pushed
the
vnc-driver
branch
from
fd5c70e to
72a87dd
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
aesteve-rh
force-pushed
the
vnc-driver
branch
from
8b06cfd to
5da9de0
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (1)
33-63: Preferanyio.sleep_forever()for blocking until cancellation.The CLI implementation is well-structured and follows the driver_click_group pattern correctly. However, line 59 uses
asyncio.Event().waitto block until interrupted. While functional,anyio.sleep_forever()is the idiomatic AnyIO approach for this use case and avoids mixing asyncio and anyio primitives unnecessarily.Apply this change:
- self.portal.call(asyncio.Event().wait) + self.portal.call(anyio.sleep_forever)This is clearer in intent (explicitly sleeping forever until cancelled) and follows AnyIO best practices.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py(2 hunks)packages/jumpstarter-driver-vnc/README.md(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/jumpstarter-driver-vnc/README.md
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Ruff should be used for code formatting and linting, excluding
jumpstarter-protocolpackage
Files:
packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use
CompositeClientfromjumpstarter_driver_composite.clientfor composite drivers with child drivers.
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver implementations should follow existing code style validated with
make lint(fix withmake lint-fix), perform static type checking withmake ty-pkg-${package_name}, add comprehensive tests, and verify all tests pass withmake test-pkg-${package_name}ormake test
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must register via the `jumpstarter.drivers` entry point in `pyproject.toml`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must follow the naming pattern `jumpstarter-driver-<name>`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py : Driver implementations should follow existing code style validated with `make lint` (fix with `make lint-fix`), perform static type checking with `make ty-pkg-${package_name}`, add comprehensive tests, and verify all tests pass with `make test-pkg-${package_name}` or `make test`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must depend on `jumpstarter` and specific hardware libraries in their `pyproject.toml`
Learnt from: bennyz
Repo: jumpstarter-dev/jumpstarter PR: 241
File: packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py:52-60
Timestamp: 2025-01-29T11:52:43.554Z
Learning: The TFTP driver (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/driver.py) handles all low-level concerns like path validation, error handling, and checksum computation. The client (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py) should remain simple as it delegates these responsibilities to the driver.
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py : Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use `CompositeClient` from `jumpstarter_driver_composite.client` for composite drivers with child drivers.
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
🧬 Code graph analysis (1)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (6)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (1)
client(17-19)packages/jumpstarter-driver-composite/jumpstarter_driver_composite/client.py (1)
CompositeClient(28-55)packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/client.py (1)
novnc(21-23)packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py (1)
NovncAdapter(13-41)packages/jumpstarter/jumpstarter/client/decorators.py (1)
driver_click_group(13-54)packages/jumpstarter/jumpstarter/client/base.py (1)
call(42-52)
⏰ 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). (11)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: build
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: e2e
🔇 Additional comments (5)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (3)
1-17: LGTM! Imports and type hints are well-structured.The imports are appropriate for a VNC driver client, including CompositeClient (per learnings), NovncAdapter, and the necessary CLI decorators. The TYPE_CHECKING guard for TCPClient is a good practice.
19-25: LGTM! Client class structure follows established patterns.The VNClient correctly extends CompositeClient (as per learnings) and provides a properly typed
tcpproperty accessor for the underlying TCPClient child.
27-31: LGTM! Session context manager is correctly implemented.The session context manager properly delegates to NovncAdapter (which is decorated with
@blocking, making it synchronous) and passes through the encrypt parameter. This follows the pattern established in the QEMU driver.packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py (2)
13-13: LGTM! Encryption parameter addition is well-designed.The new
encryptparameter with a sensible default allows callers to opt into encrypted connections while maintaining backward compatibility.
22-30: LGTM! URL construction logic is correct and clean.The dynamic scheme selection and parameter construction properly handle both encrypted and unencrypted modes. Adding
encrypt=0only when not encrypting is correct, as noVNC defaults to expecting encryption. The refactored params dictionary is more maintainable than the previous inline approach.
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @aesteve-rh. * #775 (comment) The following files were modified: * `packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py` * `packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py` * `packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py`
aesteve-rh
force-pushed
the
vnc-driver
branch
2 times, most recently
from
f2df238 to
efb2807
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
📜 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 (11)
docs/source/reference/package-apis/drivers/index.md(2 hunks)docs/source/reference/package-apis/drivers/vnc.md(1 hunks)packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py(2 hunks)packages/jumpstarter-driver-vnc/README.md(1 hunks)packages/jumpstarter-driver-vnc/examples/exporter.yaml(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py(1 hunks)packages/jumpstarter-driver-vnc/pyproject.toml(1 hunks)pyproject.toml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/jumpstarter-driver-vnc/README.md
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/init.py
- packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
- pyproject.toml
- packages/jumpstarter-driver-vnc/examples/exporter.yaml
🧰 Additional context used
📓 Path-based instructions (5)
packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use
CompositeClientfromjumpstarter_driver_composite.clientfor composite drivers with child drivers.
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver implementations should follow existing code style validated with
make lint(fix withmake lint-fix), perform static type checking withmake ty-pkg-${package_name}, add comprehensive tests, and verify all tests pass withmake test-pkg-${package_name}ormake test
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Ruff should be used for code formatting and linting, excluding
jumpstarter-protocolpackage
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.pypackages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py
packages/jumpstarter-driver-*/pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver package names should be lowercase with hyphens for multi-word names (e.g.,
my-driver,custom-power,device-controller)
packages/jumpstarter-driver-*/pyproject.toml: Driver packages must follow the naming patternjumpstarter-driver-<name>
Driver packages must register via thejumpstarter.driversentry point inpyproject.toml
Driver packages must depend onjumpstarterand specific hardware libraries in theirpyproject.toml
Files:
packages/jumpstarter-driver-vnc/pyproject.toml
packages/*/pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Each package's
pyproject.tomlmust include project metadata with Apache-2.0 license only
Files:
packages/jumpstarter-driver-vnc/pyproject.toml
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must register via the `jumpstarter.drivers` entry point in `pyproject.toml`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must follow the naming pattern `jumpstarter-driver-<name>`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py : Driver implementations should follow existing code style validated with `make lint` (fix with `make lint-fix`), perform static type checking with `make ty-pkg-${package_name}`, add comprehensive tests, and verify all tests pass with `make test-pkg-${package_name}` or `make test`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must depend on `jumpstarter` and specific hardware libraries in their `pyproject.toml`
Learnt from: bennyz
Repo: jumpstarter-dev/jumpstarter PR: 241
File: packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py:52-60
Timestamp: 2025-01-29T11:52:43.554Z
Learning: The TFTP driver (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/driver.py) handles all low-level concerns like path validation, error handling, and checksum computation. The client (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py) should remain simple as it delegates these responsibilities to the driver.
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: After driver creation, implement driver logic in `driver.py`, add tests in `driver_test.py`, update README.md with specific documentation, and test the driver
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: All driver packages should have a symlink in `docs/source/reference/package-apis/drivers/` pointing to the driver's README.md
Applied to files:
docs/source/reference/package-apis/drivers/vnc.md
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Applied to files:
docs/source/reference/package-apis/drivers/vnc.mdpackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.pypackages/jumpstarter-driver-vnc/pyproject.tomldocs/source/reference/package-apis/drivers/index.md
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
Applied to files:
docs/source/reference/package-apis/drivers/vnc.mdpackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.pypackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must follow the naming pattern `jumpstarter-driver-<name>`
Applied to files:
docs/source/reference/package-apis/drivers/vnc.mdpackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must register via the `jumpstarter.drivers` entry point in `pyproject.toml`
Applied to files:
docs/source/reference/package-apis/drivers/vnc.mdpackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must depend on `jumpstarter` and specific hardware libraries in their `pyproject.toml`
Applied to files:
docs/source/reference/package-apis/drivers/vnc.mdpackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-01-29T11:52:43.554Z
Learnt from: bennyz
Repo: jumpstarter-dev/jumpstarter PR: 241
File: packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py:52-60
Timestamp: 2025-01-29T11:52:43.554Z
Learning: The TFTP driver (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/driver.py) handles all low-level concerns like path validation, error handling, and checksum computation. The client (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py) should remain simple as it delegates these responsibilities to the driver.
Applied to files:
docs/source/reference/package-apis/drivers/vnc.mdpackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.pydocs/source/reference/package-apis/drivers/index.md
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py : Driver implementations should follow existing code style validated with `make lint` (fix with `make lint-fix`), perform static type checking with `make ty-pkg-${package_name}`, add comprehensive tests, and verify all tests pass with `make test-pkg-${package_name}` or `make test`
Applied to files:
docs/source/reference/package-apis/drivers/vnc.mdpackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.pypackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py : Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use `CompositeClient` from `jumpstarter_driver_composite.client` for composite drivers with child drivers.
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-composite/pyproject.toml : Composite drivers that have child drivers should inherit from `CompositeClient` in `jumpstarter_driver_composite.client` and have a dependency on `jumpstarter-driver-composite` in `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.pypackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-cli-*/pyproject.toml : CLI packages must depend on `jumpstarter` and `jumpstarter-cli-common` in their `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to examples/*/pyproject.toml : Example packages should depend on relevant driver packages in their `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver package names should be lowercase with hyphens for multi-word names (e.g., `my-driver`, `custom-power`, `device-controller`)
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to pyproject.toml : Top-level `pyproject.toml` should define the UV workspace and include all packages
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/*/pyproject.toml : Each package's `pyproject.toml` must include project metadata with Apache-2.0 license only
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/driver.py : Driver class names should be in CamelCase and be descriptive with appropriate suffixes based on functionality: Power drivers should end with `*Power`, Network drivers with `*Network`, Flasher drivers with `*Flasher`, Console drivers with `*Console`, Server drivers with `*Server`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-02-08T16:08:34.616Z
Learnt from: bennyz
Repo: jumpstarter-dev/jumpstarter PR: 267
File: packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/driver.py:32-40
Timestamp: 2025-02-08T16:08:34.616Z
Learning: In the TFTP driver (jumpstarter_driver_tftp), the TFTP server component is read-only, while file management operations (upload, delete) are handled through the driver's client interface, not through the TFTP protocol itself.
Applied to files:
docs/source/reference/package-apis/drivers/index.md
🧬 Code graph analysis (2)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (5)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (1)
client(23-30)packages/jumpstarter-driver-composite/jumpstarter_driver_composite/client.py (1)
CompositeClient(28-55)packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py (1)
NovncAdapter(13-57)packages/jumpstarter/jumpstarter/client/decorators.py (1)
driver_click_group(13-54)packages/jumpstarter/jumpstarter/client/base.py (1)
call(42-52)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py (5)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (2)
client(23-30)Vnc(7-30)packages/jumpstarter-driver-composite/jumpstarter_driver_composite/client.py (1)
CompositeClient(28-55)packages/jumpstarter/jumpstarter/client/base.py (1)
DriverClient(19-105)packages/jumpstarter/jumpstarter/driver/base.py (1)
Driver(57-274)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (1)
tcp(23-30)
🪛 GitHub Actions: Lint
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
[error] 3-3: F401 'asyncio' imported but unused.
🪛 GitHub Check: ruff
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
[failure] 3-3: Ruff (F401)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py:3:8: F401 asyncio imported but unused
⏰ 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). (11)
- GitHub Check: build
- GitHub Check: e2e
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
🔇 Additional comments (6)
packages/jumpstarter-driver-vnc/pyproject.toml (1)
1-48: Project configuration is well-structured and meets all driver packaging guidelines.The VNC driver package is properly configured with:
- Correct naming pattern (
jumpstarter-driver-vnc)- Apache-2.0 license
- Required dependencies on
jumpstarter, composite, and network drivers- Properly registered entry point (
jumpstarter.drivers.vnc)- Correct source_archive URL (previously flagged issue has been resolved)
- Appropriate pytest and build system configuration for async-based testing
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py (3)
1-18: Test scaffolding looks fine; FakeTcpDriver is a reasonable stub.
31-34: Missing-child validation test matchesVnc.__post_init__contract.
20-29: No changes needed to the test. Thetcpattribute inVNClientis correctly defined as a@property(not a method), soclient.tcpproperly returns the TCP client instance, and theisinstance(client.tcp, DriverClient)assertion is valid and will work as intended.Likely an incorrect or invalid review comment.
docs/source/reference/package-apis/drivers/index.md (1)
45-46: Docs index update looks consistent.Adds the driver entry and includes
vnc.mdin the toctree as expected.Also applies to: 115-116
docs/source/reference/package-apis/drivers/vnc.md (1)
1-1: The file is correctly a symlink to the driver README, as required by project structure.
packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py
Outdated
Show resolved
Hide resolved
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py (1)
13-28: Fix unaddressed docstring typo:ncrypt=0→encrypt=0.This typo was flagged in a previous review but remains unfixed. Line 23 incorrectly references
ncrypt=0in the docstring, while the code at line 46 correctly usesparams["encrypt"] = 0.Apply this diff to fix the docstring:
- encrypt (bool): If True use "https" in the generated URL; - if False use "http" and include `ncrypt=0` in the URL query. + encrypt (bool): If True use "https" in the generated URL; + if False use "http" and include `encrypt=0` in the URL query.packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (1)
32-44: Clarify docstring: method returns HTTP(S) URL, not WebSocket URL.The docstring states "
use wss://" and "use ws://", butsession()actually yields an HTTP(S) URL to the noVNC web client (e.g.,https://novnc.com/noVNC/vnc.html?...), not a WebSocket URL. Theencryptparameter controls:
- The scheme of the returned URL (
https://vshttp://)- An
encrypt=0query parameter that instructs the noVNC client (running in the browser) to usews://instead ofwss://for its WebSocket connection to the proxyUpdate the docstring to accurately describe what the method returns:
def session(self, *, encrypt: bool = False) -> typing.Iterator[str]: """ Open a noVNC session and yield the connection URL. Parameters: - encrypt (bool): If True, request an encrypted WebSocket (use `wss://`); - otherwise use `ws://`. + encrypt (bool): If True, use `https://` in the returned URL and instruct + the noVNC client to use secure WebSockets (`wss://`); + if False, use `http://` and include `encrypt=0` to request + plain WebSockets (`ws://`). Returns: url (str): The URL to connect to the VNC session. """
📜 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 (11)
docs/source/reference/package-apis/drivers/index.md(2 hunks)docs/source/reference/package-apis/drivers/vnc.md(1 hunks)packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py(2 hunks)packages/jumpstarter-driver-vnc/README.md(1 hunks)packages/jumpstarter-driver-vnc/examples/exporter.yaml(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py(1 hunks)packages/jumpstarter-driver-vnc/pyproject.toml(1 hunks)pyproject.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/jumpstarter-driver-vnc/examples/exporter.yaml
- packages/jumpstarter-driver-vnc/README.md
- docs/source/reference/package-apis/drivers/index.md
- packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py
- packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
- packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/init.py
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Ruff should be used for code formatting and linting, excluding
jumpstarter-protocolpackage
Files:
packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
packages/jumpstarter-driver-*/pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver package names should be lowercase with hyphens for multi-word names (e.g.,
my-driver,custom-power,device-controller)
packages/jumpstarter-driver-*/pyproject.toml: Driver packages must follow the naming patternjumpstarter-driver-<name>
Driver packages must register via thejumpstarter.driversentry point inpyproject.toml
Driver packages must depend onjumpstarterand specific hardware libraries in theirpyproject.toml
Files:
packages/jumpstarter-driver-vnc/pyproject.toml
packages/*/pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Each package's
pyproject.tomlmust include project metadata with Apache-2.0 license only
Files:
packages/jumpstarter-driver-vnc/pyproject.toml
packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use
CompositeClientfromjumpstarter_driver_composite.clientfor composite drivers with child drivers.
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver implementations should follow existing code style validated with
make lint(fix withmake lint-fix), perform static type checking withmake ty-pkg-${package_name}, add comprehensive tests, and verify all tests pass withmake test-pkg-${package_name}ormake test
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
pyproject.toml: Top-levelpyproject.tomlshould define the UV workspace and include all packages
All packages inpackages/*andexamples/*directories must be included as workspace members in the top-levelpyproject.toml
Files:
pyproject.toml
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must register via the `jumpstarter.drivers` entry point in `pyproject.toml`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must follow the naming pattern `jumpstarter-driver-<name>`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py : Driver implementations should follow existing code style validated with `make lint` (fix with `make lint-fix`), perform static type checking with `make ty-pkg-${package_name}`, add comprehensive tests, and verify all tests pass with `make test-pkg-${package_name}` or `make test`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must depend on `jumpstarter` and specific hardware libraries in their `pyproject.toml`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must register via the `jumpstarter.drivers` entry point in `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.tomldocs/source/reference/package-apis/drivers/vnc.mdpyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must follow the naming pattern `jumpstarter-driver-<name>`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.tomldocs/source/reference/package-apis/drivers/vnc.mdpyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must depend on `jumpstarter` and specific hardware libraries in their `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.tomldocs/source/reference/package-apis/drivers/vnc.mdpyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-cli-*/pyproject.toml : CLI packages must depend on `jumpstarter` and `jumpstarter-cli-common` in their `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.tomlpyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver package names should be lowercase with hyphens for multi-word names (e.g., `my-driver`, `custom-power`, `device-controller`)
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to examples/*/pyproject.toml : Example packages should depend on relevant driver packages in their `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to pyproject.toml : Top-level `pyproject.toml` should define the UV workspace and include all packages
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.tomlpyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.tomldocs/source/reference/package-apis/drivers/vnc.md
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/*/pyproject.toml : Each package's `pyproject.toml` must include project metadata with Apache-2.0 license only
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py : Driver implementations should follow existing code style validated with `make lint` (fix with `make lint-fix`), perform static type checking with `make ty-pkg-${package_name}`, add comprehensive tests, and verify all tests pass with `make test-pkg-${package_name}` or `make test`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.tomldocs/source/reference/package-apis/drivers/vnc.md
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/driver.py : Driver class names should be in CamelCase and be descriptive with appropriate suffixes based on functionality: Power drivers should end with `*Power`, Network drivers with `*Network`, Flasher drivers with `*Flasher`, Console drivers with `*Console`, Server drivers with `*Server`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.tomldocs/source/reference/package-apis/drivers/vnc.mdpackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-composite/pyproject.toml : Composite drivers that have child drivers should inherit from `CompositeClient` in `jumpstarter_driver_composite.client` and have a dependency on `jumpstarter-driver-composite` in `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.tomlpackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: All driver packages should have a symlink in `docs/source/reference/package-apis/drivers/` pointing to the driver's README.md
Applied to files:
docs/source/reference/package-apis/drivers/vnc.md
📚 Learning: 2025-01-29T11:52:43.554Z
Learnt from: bennyz
Repo: jumpstarter-dev/jumpstarter PR: 241
File: packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py:52-60
Timestamp: 2025-01-29T11:52:43.554Z
Learning: The TFTP driver (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/driver.py) handles all low-level concerns like path validation, error handling, and checksum computation. The client (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py) should remain simple as it delegates these responsibilities to the driver.
Applied to files:
docs/source/reference/package-apis/drivers/vnc.md
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py : Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use `CompositeClient` from `jumpstarter_driver_composite.client` for composite drivers with child drivers.
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to **/*.py : Ruff should be used for code formatting and linting, excluding `jumpstarter-protocol` package
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Core packages must depend on `jumpstarter-protocol`
Applied to files:
pyproject.toml
📚 Learning: 2025-09-06T05:25:18.184Z
Learnt from: michalskrivanek
Repo: jumpstarter-dev/jumpstarter PR: 608
File: packages/jumpstarter-cli/jumpstarter_cli/run.py:0-0
Timestamp: 2025-09-06T05:25:18.184Z
Learning: Zombie process reaping in jumpstarter is useful beyond just PID 1 cases, as drivers can leave behind processes in various execution contexts.
Applied to files:
pyproject.toml
📚 Learning: 2025-09-06T05:25:18.184Z
Learnt from: michalskrivanek
Repo: jumpstarter-dev/jumpstarter PR: 608
File: packages/jumpstarter-cli/jumpstarter_cli/run.py:0-0
Timestamp: 2025-09-06T05:25:18.184Z
Learning: The Shell driver in jumpstarter can spawn multiple processes and leave them behind, creating zombie processes that need to be reaped even in non-PID 1 scenarios.
Applied to files:
pyproject.toml
📚 Learning: 2025-10-05T07:18:21.352Z
Learnt from: bennyz
Repo: jumpstarter-dev/jumpstarter PR: 661
File: packages/jumpstarter-driver-iscsi/jumpstarter_driver_iscsi/driver.py:391-397
Timestamp: 2025-10-05T07:18:21.352Z
Learning: The iSCSI driver in `packages/jumpstarter-driver-iscsi/jumpstarter_driver_iscsi/driver.py` is designed to support only one LUN at a time, so hardcoding LUN ID 0 in the `add_lun` method is acceptable.
Applied to files:
pyproject.toml
⏰ 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). (6)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: e2e
🔇 Additional comments (3)
docs/source/reference/package-apis/drivers/vnc.md (1)
1-1: LGTM! Documentation symlink follows the required pattern.The symlink correctly points to the VNC driver's README and follows the established convention for driver documentation.
Based on learnings, all driver packages should have a symlink in this location pointing to the driver's README.
packages/jumpstarter-driver-vnc/pyproject.toml (1)
1-48: LGTM! Package metadata follows all driver packaging guidelines.The pyproject.toml correctly follows the required patterns:
- Package name:
jumpstarter-driver-vnc✓- Entry point registration:
jumpstarter.drivers.vnc✓- Required dependencies: jumpstarter, jumpstarter-driver-composite, jumpstarter-driver-network ✓
- Apache-2.0 license ✓
- Correct VCS URLs ✓
As per coding guidelines and learnings, the package metadata and entry-point setup are correctly configured.
pyproject.toml (1)
38-38: LGTM! Workspace registration follows the required pattern.The new VNC driver is correctly registered in the workspace following the same pattern as other driver packages.
As per coding guidelines, all packages must be included as workspace members in the top-level pyproject.toml.
aesteve-rh
force-pushed
the
vnc-driver
branch
from
efb2807 to
ec22e95
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (3)
31-42: Docstring still conflates “encrypted WebSocket” with returned HTTP(S) URL.
Sincesession()yields a noVNC page URL, describe it as: “use HTTPS for the noVNC page, and request secure WebSockets via noVNC’sencryptflag” (rather than implying the returned URL is a WebSocket URL).
59-72: Remove remainingwss://reference in the group docstring (misleading).
Line 71 still says “encrypted (wss://) connection” but the command prints an HTTP(S) URL to the noVNC web client.
93-106: Verifyself.portal.call(anyio.sleep_forever)cancellation +self.portalavailability.
This will only be reliable ifself.portalis always initialized here, and if Ctrl+C results in eitherKeyboardInterruptin this thread or an AnyIO cancellation exception fromportal.call.#!/bin/bash set -euo pipefail # 1) Confirm where `portal` is defined/initialized (and whether it's always present) rg -n --hidden --no-ignore-vcs -S '\bportal\b' packages/jumpstarter/jumpstarter/client packages/jumpstarter-driver-composite packages/jumpstarter-driver-vnc # 2) Confirm DriverClient / CompositeClient hierarchy and whether cli paths construct a portal rg -n --hidden --no-ignore-vcs -S 'class (DriverClient|CompositeClient)\b|def cli\(|start_blocking_portal|BlockingPortal' \ packages/jumpstarter/jumpstarter/client packages/jumpstarter-driver-composite packages/jumpstarter-driver-vncAnyIO: in a BlockingPortal, what does `portal.call(anyio.sleep_forever)` do on Ctrl+C, and what exception(s) should be expected/handled? Also, is `anyio.sleep_forever()` the recommended primitive for "block until cancelled" in AnyIO 4.x?
🧹 Nitpick comments (1)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (1)
7-21: Consider renamingVncto match driver naming conventions (e.g.,VncServer).
Current name is clear, but it doesn’t follow the “*Server” suffix guidance for server drivers. As per coding guidelines, please confirm the intended category and naming.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py(2 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
packages/jumpstarter-driver-*/jumpstarter_driver_*/driver.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver class names should be in CamelCase and be descriptive with appropriate suffixes based on functionality: Power drivers should end with
*Power, Network drivers with*Network, Flasher drivers with*Flasher, Console drivers with*Console, Server drivers with*Server
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver implementations should follow existing code style validated with
make lint(fix withmake lint-fix), perform static type checking withmake ty-pkg-${package_name}, add comprehensive tests, and verify all tests pass withmake test-pkg-${package_name}ormake test
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Ruff should be used for code formatting and linting, excluding
jumpstarter-protocolpackage
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.pypackages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py
packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use
CompositeClientfromjumpstarter_driver_composite.clientfor composite drivers with child drivers.
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must register via the `jumpstarter.drivers` entry point in `pyproject.toml`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py : Driver implementations should follow existing code style validated with `make lint` (fix with `make lint-fix`), perform static type checking with `make ty-pkg-${package_name}`, add comprehensive tests, and verify all tests pass with `make test-pkg-${package_name}` or `make test`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must follow the naming pattern `jumpstarter-driver-<name>`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must depend on `jumpstarter` and specific hardware libraries in their `pyproject.toml`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py : Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use `CompositeClient` from `jumpstarter_driver_composite.client` for composite drivers with child drivers.
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-composite/pyproject.toml : Composite drivers that have child drivers should inherit from `CompositeClient` in `jumpstarter_driver_composite.client` and have a dependency on `jumpstarter-driver-composite` in `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to **/*.py : Ruff should be used for code formatting and linting, excluding `jumpstarter-protocol` package
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
🧬 Code graph analysis (2)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (2)
packages/jumpstarter/jumpstarter/driver/base.py (1)
Driver(57-274)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py (1)
client(16-17)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (5)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (1)
client(23-30)packages/jumpstarter-driver-composite/jumpstarter_driver_composite/client.py (1)
CompositeClient(28-55)packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/client.py (1)
novnc(21-23)packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py (1)
NovncAdapter(13-57)packages/jumpstarter/jumpstarter/client/decorators.py (1)
driver_click_group(13-54)
⏰ 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). (10)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: build
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: e2e
🔇 Additional comments (3)
packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py (1)
13-28: Docstring/behavior alignment looks good (encrypt=0 typo fixed).
Theencrypt=0wording now matches the actual query parameter written toparams.packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (1)
22-30:client()path looks correct and consistent with Driver’s contract.
Returning"jumpstarter_driver_vnc.client.VNClient"is the expected dotted import path style.packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (1)
21-29:tcpproperty is a clean composite alias.
Clear and type-safe enough (with TYPE_CHECKING + cast).
packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py
Show resolved
Hide resolved
mangelajo
left a comment
mangelajo
left a comment
There was a problem hiding this comment.
Just a bunch of nitpicks. But it's a great addition! thank you @aesteve-rh !
packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py
Show resolved
Hide resolved
| bridged to the local listener. | ||
| method (str): Name of the async stream method to call on the client (default "connect"). | ||
| encrypt (bool): If True use "https" in the generated URL; | ||
| if False use "http" and include `encrypt=0` in the URL query. |
There was a problem hiding this comment.
what is the encrypt=1/encrypt=0 meaning to noVNC in contrast to https/http? I was looking at noVNC docs and I don't find it.
There was a problem hiding this comment.
Here's the mention to encrypt in the noVNC docs: https://novnc.com/noVNC/docs/EMBEDDING.html
To be honest, when I run with https and encrypt=0, it works. So I could leave the scheme part of the url unchanged. But I thought that it was more correct to not mix secure origin from the browser (https) with unencrypted connection? At least, I think it makes sense?
Otherwise, I can revert the https/http part of the change.
There was a problem hiding this comment.
Also, I could perhaps change the defaults to true, to keep the current behaviour.
There was a problem hiding this comment.
Oh, and another idea, could be adding a setting to the driver (on the exporter) to decide what is the default when user uses the cli.
I.e. you probably know if VNC will be encrypted or not... so you can set the default of the encrypted flag.
That would probably require you to split the click bools into two:
- no flag set: use whatever the driver config on the exporter said
- --encryption: use encryption
- --no-encryption: don't use encryption
There was a problem hiding this comment.
I liked the idea. Done!
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
Outdated
Show resolved
Hide resolved
Create a vnc driver for jumpstarter that opens a tunnel to connect vnc clients locally (and opens a browser directly to that tunnel). If a VNC server is running in the remote machine it will allow sharing the screen. Co-Authored-By: Claude noreply@anthropic.com Signed-off-by: Albert Esteve <aesteve@redhat.com>
aesteve-rh
force-pushed
the
vnc-driver
branch
from
ec22e95 to
021de5d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py (1)
13-54:encrypt=Falsewon't work with the https-served noVNC page (browsers block mixed-content ws://).The
--no-encryptflag is exposed as a user-facing CLI option (line 96 in VNC client), but the implementation has a critical flaw: the noVNC URL is always generated withhttps://novnc.com/...regardless of the encrypt parameter value. When users requestencrypt=False, the page loads over HTTPS but attempts to establish aws://WebSocket (unencrypted), which browsers block as mixed content.Consider one of:
- Force
encrypt=1when the URL targetsnovnc.comover HTTPS (and disable/remove--no-encryptfor that path), or- Serve noVNC assets locally so you can use
http://localhost/...+ws://...safely.
♻️ Duplicate comments (1)
packages/jumpstarter-driver-vnc/README.md (1)
36-47: Fixvnc.session()example to match the actual yield type (URL string vs adapter object).
The README showsnovnc_adapter.urlandawait novnc_adapter.wait(), but the current implementation description suggestsvnc.session()yields a URL (string). Please align docs with the real return/yield type to avoid copy/paste breakage.-### `vnc.session()` - -This asynchronous context manager establishes a connection to the remote VNC server and provides a local web server to view the session. +### `vnc.session()` + +This async context manager establishes a connection to the remote VNC server and yields the local noVNC URL. **Usage:** ```python -async with vnc.session() as novnc_adapter: - print(f"VNC session available at: {novnc_adapter.url}") - # The session remains open until the context block is exited. - await novnc_adapter.wait() +async with vnc.session() as url: + print(f"VNC session available at: {url}") + # Keep the context open while you use the session. + ...</blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (3)</summary><blockquote> <details> <summary>packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.py (1)</summary><blockquote> `1-3`: **Remove the redundant self-alias and explicitly declare exports.** `VNClient = VNClient` is a no-op and can be dropped in favor of `__all__`. ```diff from .client import VNClient -VNClient = VNClient +__all__ = ["VNClient"]packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (1)
11-46: Consider renamingVnc→VncServerto match driver naming guidelines.
If this is considered a “server” driver in this repo, the*Serversuffix would align with the convention.@dataclass -class Vnc(Composite): +class VncServer(Composite): @@ - raise ConfigurationError("A tcp child is required for Vnc") + raise ConfigurationError("A tcp child is required for VncServer") @@ - return "jumpstarter_driver_vnc.client.VNClient" + return "jumpstarter_driver_vnc.client.VNClient"packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (1)
71-84: Clarify command group docstring.The docstring for the
vnc()group function (lines 73-84) describes the behavior of thesessionsubcommand rather than the command group itself. This is confusing since thesessionsubcommand already has its own docstring at lines 102-112.Apply this diff to clarify:
@driver_click_group(self) def vnc(): """ - Open a VNC session and block until the user closes it. - - When invoked, prints the connection URL for the noVNC session, optionally - opens that URL in the user's web browser, and waits for user-initiated - termination (for example, Ctrl+C). On exit, prints a message indicating - the session is closing. - - Parameters: - browser (bool): If True, open the session URL in the default web browser. - encrypt_override (bool): If True, request an encrypted vnc connection. + VNC client commands for managing VNC sessions. """ + pass
📜 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 (11)
docs/source/reference/package-apis/drivers/index.md(2 hunks)docs/source/reference/package-apis/drivers/vnc.md(1 hunks)packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py(2 hunks)packages/jumpstarter-driver-vnc/README.md(1 hunks)packages/jumpstarter-driver-vnc/examples/exporter.yaml(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py(1 hunks)packages/jumpstarter-driver-vnc/pyproject.toml(1 hunks)pyproject.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py
- docs/source/reference/package-apis/drivers/vnc.md
- docs/source/reference/package-apis/drivers/index.md
- packages/jumpstarter-driver-vnc/examples/exporter.yaml
🧰 Additional context used
📓 Path-based instructions (7)
packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver implementations should follow existing code style validated with
make lint(fix withmake lint-fix), perform static type checking withmake ty-pkg-${package_name}, add comprehensive tests, and verify all tests pass withmake test-pkg-${package_name}ormake test
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Ruff should be used for code formatting and linting, excluding
jumpstarter-protocolpackage
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.pypackages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
packages/jumpstarter-driver-*/jumpstarter_driver_*/driver.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver class names should be in CamelCase and be descriptive with appropriate suffixes based on functionality: Power drivers should end with
*Power, Network drivers with*Network, Flasher drivers with*Flasher, Console drivers with*Console, Server drivers with*Server
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
pyproject.toml: Top-levelpyproject.tomlshould define the UV workspace and include all packages
All packages inpackages/*andexamples/*directories must be included as workspace members in the top-levelpyproject.toml
Files:
pyproject.toml
packages/jumpstarter-driver-*/pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver package names should be lowercase with hyphens for multi-word names (e.g.,
my-driver,custom-power,device-controller)
packages/jumpstarter-driver-*/pyproject.toml: Driver packages must follow the naming patternjumpstarter-driver-<name>
Driver packages must register via thejumpstarter.driversentry point inpyproject.toml
Driver packages must depend onjumpstarterand specific hardware libraries in theirpyproject.toml
Files:
packages/jumpstarter-driver-vnc/pyproject.toml
packages/*/pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Each package's
pyproject.tomlmust include project metadata with Apache-2.0 license only
Files:
packages/jumpstarter-driver-vnc/pyproject.toml
packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use
CompositeClientfromjumpstarter_driver_composite.clientfor composite drivers with child drivers.
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must register via the `jumpstarter.drivers` entry point in `pyproject.toml`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must follow the naming pattern `jumpstarter-driver-<name>`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py : Driver implementations should follow existing code style validated with `make lint` (fix with `make lint-fix`), perform static type checking with `make ty-pkg-${package_name}`, add comprehensive tests, and verify all tests pass with `make test-pkg-${package_name}` or `make test`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must depend on `jumpstarter` and specific hardware libraries in their `pyproject.toml`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.pypackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.pypackages/jumpstarter-driver-vnc/README.mdpackages/jumpstarter-driver-vnc/pyproject.tomlpackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
📚 Learning: 2025-01-29T11:52:43.554Z
Learnt from: bennyz
Repo: jumpstarter-dev/jumpstarter PR: 241
File: packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py:52-60
Timestamp: 2025-01-29T11:52:43.554Z
Learning: The TFTP driver (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/driver.py) handles all low-level concerns like path validation, error handling, and checksum computation. The client (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py) should remain simple as it delegates these responsibilities to the driver.
Applied to files:
packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.pypackages/jumpstarter-driver-vnc/README.md
📚 Learning: 2025-02-08T16:08:34.616Z
Learnt from: bennyz
Repo: jumpstarter-dev/jumpstarter PR: 267
File: packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/driver.py:32-40
Timestamp: 2025-02-08T16:08:34.616Z
Learning: In the TFTP driver (jumpstarter_driver_tftp), the TFTP server component is read-only, while file management operations (upload, delete) are handled through the driver's client interface, not through the TFTP protocol itself.
Applied to files:
packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-composite/pyproject.toml : Composite drivers that have child drivers should inherit from `CompositeClient` in `jumpstarter_driver_composite.client` and have a dependency on `jumpstarter-driver-composite` in `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.pypackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py : Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use `CompositeClient` from `jumpstarter_driver_composite.client` for composite drivers with child drivers.
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.pypackages/jumpstarter-driver-vnc/README.mdpackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Applied to files:
packages/jumpstarter-driver-vnc/README.mdpackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to pyproject.toml : Top-level `pyproject.toml` should define the UV workspace and include all packages
Applied to files:
pyproject.tomlpackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must register via the `jumpstarter.drivers` entry point in `pyproject.toml`
Applied to files:
pyproject.tomlpackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must follow the naming pattern `jumpstarter-driver-<name>`
Applied to files:
pyproject.tomlpackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must depend on `jumpstarter` and specific hardware libraries in their `pyproject.toml`
Applied to files:
pyproject.tomlpackages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Core packages must depend on `jumpstarter-protocol`
Applied to files:
pyproject.toml
📚 Learning: 2025-09-06T05:25:18.184Z
Learnt from: michalskrivanek
Repo: jumpstarter-dev/jumpstarter PR: 608
File: packages/jumpstarter-cli/jumpstarter_cli/run.py:0-0
Timestamp: 2025-09-06T05:25:18.184Z
Learning: Zombie process reaping in jumpstarter is useful beyond just PID 1 cases, as drivers can leave behind processes in various execution contexts.
Applied to files:
pyproject.toml
📚 Learning: 2025-09-06T05:25:18.184Z
Learnt from: michalskrivanek
Repo: jumpstarter-dev/jumpstarter PR: 608
File: packages/jumpstarter-cli/jumpstarter_cli/run.py:0-0
Timestamp: 2025-09-06T05:25:18.184Z
Learning: The Shell driver in jumpstarter can spawn multiple processes and leave them behind, creating zombie processes that need to be reaped even in non-PID 1 scenarios.
Applied to files:
pyproject.toml
📚 Learning: 2025-10-05T07:18:21.352Z
Learnt from: bennyz
Repo: jumpstarter-dev/jumpstarter PR: 661
File: packages/jumpstarter-driver-iscsi/jumpstarter_driver_iscsi/driver.py:391-397
Timestamp: 2025-10-05T07:18:21.352Z
Learning: The iSCSI driver in `packages/jumpstarter-driver-iscsi/jumpstarter_driver_iscsi/driver.py` is designed to support only one LUN at a time, so hardcoding LUN ID 0 in the `add_lun` method is acceptable.
Applied to files:
pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-cli-*/pyproject.toml : CLI packages must depend on `jumpstarter` and `jumpstarter-cli-common` in their `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to examples/*/pyproject.toml : Example packages should depend on relevant driver packages in their `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver package names should be lowercase with hyphens for multi-word names (e.g., `my-driver`, `custom-power`, `device-controller`)
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/*/pyproject.toml : Each package's `pyproject.toml` must include project metadata with Apache-2.0 license only
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py : Driver implementations should follow existing code style validated with `make lint` (fix with `make lint-fix`), perform static type checking with `make ty-pkg-${package_name}`, add comprehensive tests, and verify all tests pass with `make test-pkg-${package_name}` or `make test`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/driver.py : Driver class names should be in CamelCase and be descriptive with appropriate suffixes based on functionality: Power drivers should end with `*Power`, Network drivers with `*Network`, Flasher drivers with `*Flasher`, Console drivers with `*Console`, Server drivers with `*Server`
Applied to files:
packages/jumpstarter-driver-vnc/pyproject.toml
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to **/*.py : Ruff should be used for code formatting and linting, excluding `jumpstarter-protocol` package
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
🧬 Code graph analysis (3)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/__init__.py (3)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (1)
client(39-46)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py (1)
client(16-17)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (1)
VNClient(19-128)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (4)
packages/jumpstarter-cli-driver/jumpstarter_cli_driver/__init__.py (1)
driver(11-12)packages/jumpstarter-driver-composite/jumpstarter_driver_composite/driver.py (1)
Composite(17-18)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (1)
get_default_encrypt(54-56)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py (1)
client(16-17)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (6)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (1)
client(39-46)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py (1)
client(16-17)packages/jumpstarter-driver-composite/jumpstarter_driver_composite/client.py (1)
CompositeClient(28-55)packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py (1)
NovncAdapter(13-54)packages/jumpstarter/jumpstarter/client/decorators.py (1)
driver_click_group(13-54)packages/jumpstarter/jumpstarter/client/core.py (1)
call_async(79-103)
⏰ 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). (11)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: build
- GitHub Check: e2e
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
🔇 Additional comments (3)
pyproject.toml (1)
38-38: Workspace source registration looks correct.
Addingjumpstarter-driver-vncunder[tool.uv.sources]is consistent with the existing workspace mapping.packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (1)
33-37: No action required. The driver package is correctly configured: entry pointvncis properly registered in[project.entry-points."jumpstarter.drivers"]pointing to theVncclass, and all required dependencies are present (jumpstarter,jumpstarter-driver-composite, andjumpstarter-driver-network).packages/jumpstarter-driver-vnc/pyproject.toml (1)
1-48: LGTM! Project configuration follows all guidelines.The pyproject.toml correctly follows all coding guidelines:
- Package name follows pattern:
jumpstarter-driver-vnc✓- Apache-2.0 license specified ✓
- Required dependencies present:
jumpstarter,jumpstarter-driver-composite,jumpstarter-driver-network✓- Entry point registered via
jumpstarter.drivers✓- Build system and test configuration properly set up ✓
Based on learnings, this aligns with driver package requirements.
aesteve-rh
force-pushed
the
vnc-driver
branch
from
021de5d to
c178d57
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py (2)
10-11:encryptbehavior looks internally inconsistent (https page vs ws/wss to local listener)
Withyieldalways returning anhttps://novnc.com/...URL (Line 45-47),encrypt=0(Line 36) will typically make noVNC attemptws://host:port, which browsers commonly block as mixed-content when the page is HTTPS. Conversely,encrypt=1implieswss://host:port, which will fail unless your localTemporaryTcpListener+WebsocketServerStreamendpoint actually speaks TLS.This likely needs a decision:
- either support
encrypt=Falseby also using anhttp://origin (and ensure novnc.com won’t force-redirect to HTTPS), or- force encrypted mode only and add TLS support to the local websocket endpoint, or
- stop depending on novnc.com and serve noVNC locally (then you control http/https and ws/wss end-to-end).
Proposed minimal alignment (if you truly intend to support
encrypt=False):@@ - yield urlunparse( + scheme = "https" if encrypt else "http" + yield urlunparse( ( - "https", + scheme, "novnc.com", "/noVNC/vnc.html", "", urlencode(params), "", ) )Also applies to: 34-51
35-41: Be explicit about leakinghost/portto a third-party in query params
You’re sending the local listenerhost/port(Lines 39-40) tonovnc.comvia the query string (Line 49). If this is intentional, it deserves an explicit note in docs/README; otherwise consider serving noVNC locally to avoid third-party exposure.Also applies to: 49-49
🧹 Nitpick comments (2)
packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py (1)
29-32: Avoid shadowingstreaminside nestedasync with
The inneras stream(Line 30) hides the outerstream(Line 29), making the bridging logic harder to follow.@@ - async with client.stream_async(method) as stream: - async with WebsocketServerStream(stream=stream) as stream: - async with forward_stream(conn, stream): + async with client.stream_async(method) as remote_stream: + async with WebsocketServerStream(stream=remote_stream) as ws_stream: + async with forward_stream(conn, ws_stream): passpackages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (1)
56-59:get_default_encrypt()can likely be simplified toself.call(...)
SinceDriverClient.call()already routes through the portal, you probably don’t need the unboundDriverClient.call_asyncinvocation here.@@ def get_default_encrypt(self) -> bool: """Fetch the default encryption setting from the remote driver.""" - return self.portal.call(DriverClient.call_async, self, "get_default_encrypt") + return typing.cast(bool, self.call("get_default_encrypt"))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py(2 hunks)packages/jumpstarter-driver-vnc/README.md(1 hunks)packages/jumpstarter-driver-vnc/examples/exporter.yaml(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/jumpstarter-driver-vnc/README.md
- packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
- packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py
- packages/jumpstarter-driver-vnc/examples/exporter.yaml
🧰 Additional context used
📓 Path-based instructions (3)
packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use
CompositeClientfromjumpstarter_driver_composite.clientfor composite drivers with child drivers.
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver implementations should follow existing code style validated with
make lint(fix withmake lint-fix), perform static type checking withmake ty-pkg-${package_name}, add comprehensive tests, and verify all tests pass withmake test-pkg-${package_name}ormake test
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Ruff should be used for code formatting and linting, excluding
jumpstarter-protocolpackage
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.pypackages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must register via the `jumpstarter.drivers` entry point in `pyproject.toml`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must follow the naming pattern `jumpstarter-driver-<name>`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py : Driver implementations should follow existing code style validated with `make lint` (fix with `make lint-fix`), perform static type checking with `make ty-pkg-${package_name}`, add comprehensive tests, and verify all tests pass with `make test-pkg-${package_name}` or `make test`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must depend on `jumpstarter` and specific hardware libraries in their `pyproject.toml`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py : Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use `CompositeClient` from `jumpstarter_driver_composite.client` for composite drivers with child drivers.
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to **/*.py : Ruff should be used for code formatting and linting, excluding `jumpstarter-protocol` package
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
📚 Learning: 2025-01-29T11:52:43.554Z
Learnt from: bennyz
Repo: jumpstarter-dev/jumpstarter PR: 241
File: packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py:52-60
Timestamp: 2025-01-29T11:52:43.554Z
Learning: The TFTP driver (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/driver.py) handles all low-level concerns like path validation, error handling, and checksum computation. The client (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py) should remain simple as it delegates these responsibilities to the driver.
Applied to files:
packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py
📚 Learning: 2025-02-08T16:08:34.616Z
Learnt from: bennyz
Repo: jumpstarter-dev/jumpstarter PR: 267
File: packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/driver.py:32-40
Timestamp: 2025-02-08T16:08:34.616Z
Learning: In the TFTP driver (jumpstarter_driver_tftp), the TFTP server component is read-only, while file management operations (upload, delete) are handled through the driver's client interface, not through the TFTP protocol itself.
Applied to files:
packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py
🧬 Code graph analysis (1)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (3)
packages/jumpstarter-driver-composite/jumpstarter_driver_composite/client.py (1)
CompositeClient(28-55)packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py (1)
NovncAdapter(11-52)packages/jumpstarter/jumpstarter/client/base.py (2)
DriverClient(19-105)call(42-52)
⏰ 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). (11)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: build
- GitHub Check: e2e
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
🔇 Additional comments (2)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py (2)
40-55:session()correctly enters the asyncNovncAdapterfrom sync code
Usingself.portal.wrap_async_context_manager(...)(Line 51-53) is the right pattern here.
121-131: Verify Ctrl+C reliably interruptsportal.call(anyio.sleep_forever)
This is a good idea to keep the async context alive without busy-waiting, but please sanity-check that a single Ctrl+C consistently exits on the first attempt (and doesn’t require repeated signals) in your target environments.Suggested manual checks:
- run
vnc session --browser/--no-browserand ensure it prints URL and stays open- press Ctrl+C once and ensure it prints “Closing…” and exits promptly
- confirm the adapter/streams actually close (no lingering process/ports)
Add encrypt parameter to novnc adapter class, so that it can be configured in the url params. This allows the VNC driver to set --encrypt/--no-encrypt parameter through command line (default False) for more flexibility on the type of connection the user needs. Signed-off-by: Albert Esteve <aesteve@redhat.com>
Signed-off-by: Albert Esteve <aesteve@redhat.com>
aesteve-rh
force-pushed
the
vnc-driver
branch
from
c178d57 to
6cdcf5c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (2)
11-19: Driver class name likely should follow the repo’s suffix convention (e.g.,VncConsole/VncServer/VncNetwork)Per the driver naming guidelines, consider renaming
Vncto a more specific*Console/*Server/*Networkname (whichever best matches the driver’s role). As per coding guidelines, ...
21-32: Make the configuration error more actionable (include expected child + available keys)Consider including the configured child keys in the exception message (and maybe “VNC” capitalization), to speed up debugging when misconfigured.
As per coding guidelines, ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py(2 hunks)packages/jumpstarter-driver-vnc/README.md(1 hunks)packages/jumpstarter-driver-vnc/examples/exporter.yaml(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py(1 hunks)packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver_test.py
- packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/novnc.py
- packages/jumpstarter-driver-vnc/examples/exporter.yaml
- packages/jumpstarter-driver-vnc/README.md
- packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/client.py
🧰 Additional context used
📓 Path-based instructions (3)
packages/jumpstarter-driver-*/jumpstarter_driver_*/driver.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver class names should be in CamelCase and be descriptive with appropriate suffixes based on functionality: Power drivers should end with
*Power, Network drivers with*Network, Flasher drivers with*Flasher, Console drivers with*Console, Server drivers with*Server
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver implementations should follow existing code style validated with
make lint(fix withmake lint-fix), perform static type checking withmake ty-pkg-${package_name}, add comprehensive tests, and verify all tests pass withmake test-pkg-${package_name}ormake test
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Ruff should be used for code formatting and linting, excluding
jumpstarter-protocolpackage
Files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must register via the `jumpstarter.drivers` entry point in `pyproject.toml`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must follow the naming pattern `jumpstarter-driver-<name>`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py : Driver implementations should follow existing code style validated with `make lint` (fix with `make lint-fix`), perform static type checking with `make ty-pkg-${package_name}`, add comprehensive tests, and verify all tests pass with `make test-pkg-${package_name}` or `make test`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/pyproject.toml : Driver packages must depend on `jumpstarter` and specific hardware libraries in their `pyproject.toml`
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-composite/pyproject.toml : Composite drivers that have child drivers should inherit from `CompositeClient` in `jumpstarter_driver_composite.client` and have a dependency on `jumpstarter-driver-composite` in `pyproject.toml`
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py : Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use `CompositeClient` from `jumpstarter_driver_composite.client` for composite drivers with child drivers.
Applied to files:
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py
⏰ 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). (11)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: build
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: e2e
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
🔇 Additional comments (2)
packages/jumpstarter-driver-vnc/jumpstarter_driver_vnc/driver.py (2)
33-37: Confirm@exportexpectsasynchere; otherwise prefer a sync method
get_default_encryptisasyncbut has no awaits. If the RPC/export layer supports sync exports, making it sync avoids needless coroutine overhead; if it requiresasync, keep as-is.
As per coding guidelines, ...
38-46: The dotted client pathjumpstarter_driver_vnc.client.VNClientis correct and matches the actual client symbol defined in the codebase. TheVNClientclass exists at line 18 ofjumpstarter_driver_vnc/client.py, and the driver'stcpchild requirement is properly validated and used throughout the implementation. No issues found.
|
Successfully created backport PR for |
2 participants
Create a vnc driver for jumpstarter that opens a tunnel to connect vnc clients locally (and opens a browser directly to that tunnel). If a VNC server is running in the remote machine it will allow sharing the screen.
Co-Authored-By: Claude noreply@anthropic.com
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.