Skip to content
This repository was archived by the owner on Jan 23, 2026. It is now read-only.

server-side proxy calls#705

Merged
mangelajo merged 1 commit intojumpstarter-dev:mainfrom
michalskrivanek:proxy
Oct 14, 2025
Merged

server-side proxy calls#705
mangelajo merged 1 commit intojumpstarter-dev:mainfrom
michalskrivanek:proxy

Conversation

@michalskrivanek
Copy link
Copy Markdown
Contributor

@michalskrivanek michalskrivanek commented Oct 14, 2025

When using a ref: that is internally used by another driver on the server side, forward the calls to the target driver.

Summary by CodeRabbit

  • New Features

    • Proxy now forwards attribute access to its resolved target and caches that resolution for faster repeated use.
  • Bug Fixes

    • Reporting and enumeration consistently delegate to the resolved target and surface clear errors when a target is unresolved or invalid.
  • Refactor

    • Proxy resolution and delegation behavior simplified; proxy client access now raises an explicit error to direct use via the resolved target.
  • Tests

    • Added mock drivers and scenarios validating proxy delegation and parent-to-child interactions.

@netlify
Copy link
Copy Markdown

netlify Bot commented Oct 14, 2025

Deploy Preview for jumpstarter-docs ready!

Name Link
🔨 Latest commit ba567ee
🔍 Latest deploy log https://app.netlify.com/projects/jumpstarter-docs/deploys/68ee1b2c24121f0008af35a9
😎 Deploy Preview https://deploy-preview-705--jumpstarter-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 14, 2025

Walkthrough

Adds cached proxy-target resolution to Proxy, centralizes resolution in _resolve_proxy_target, updates delegation for report, enumerate, and attribute access, changes client() to raise NotImplementedError, and adds test doubles (MockSerial, MockParent) exercising proxy forwarding and parent-to-child delegation.

Changes

Cohort / File(s) Summary
Proxy target resolution & delegation
packages/jumpstarter-driver-composite/jumpstarter_driver_composite/driver.py
Adds _proxy_target cache and _resolve_proxy_target(self, root, name); updates report(self, *, parent=None, name=None) and enumerate(...) to resolve then delegate; adds __getattr__ to forward attributes to resolved target; changes client() to raise NotImplementedError; enforces error when target unresolved at call time.
Test doubles & tests
packages/jumpstarter-driver-composite/jumpstarter_driver_composite/driver_test.py
Introduces MockSerial and MockParent (subclassing Driver), with exported methods connect, read on MockSerial and initialize on MockParent; adds tests validating proxy method forwarding and parent→child delegation.
Driver API signature change
packages/jumpstarter/jumpstarter/driver/base.py
Removed root parameter from Driver.report(...); signature now report(self, *, parent=None, name=None) and report no longer constructs or references a root.

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant Proxy
    participant Resolver as _resolve_proxy_target
    participant Target

    Caller->>Proxy: call report()/enumerate()/attribute
    Proxy->>Resolver: resolve(root?, name)
    Note over Resolver: compute target from ref\ncache to _proxy_target
    Resolver-->>Proxy: resolved Target (cached)
    Proxy->>Target: delegate method / attribute access
    Target-->>Proxy: result / value
    Proxy-->>Caller: result / value
Loading
sequenceDiagram
    participant Test as Test/Parent.initialize()
    participant Parent
    participant ChildProxy as children["serial"] (Proxy)
    participant Resolver as _resolve_proxy_target
    participant Serial as MockSerial

    Test->>Parent: initialize()
    Parent->>ChildProxy: connect()
    ChildProxy->>Resolver: resolve(self or root, "serial")
    Resolver-->>ChildProxy: MockSerial (cached)
    ChildProxy->>Serial: connect()
    Serial-->>ChildProxy: "connected"
    ChildProxy-->>Parent: return "connected"
    Parent-->>Test: return
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • mangelajo

Poem

I hop along the call-chain trail,
Caching carrots so calls won't fail.
A parent calls, a child replies,
Resolved paths bright before my eyes.
Thump-thump—delegation, snug and hale. 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 26.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly captures the core enhancement of forwarding proxy calls on the server side and aligns with the PR’s purpose of adding lazy resolution and delegation in the Proxy implementation. It is concise, clearly related to the main changeset, and avoids unnecessary detail or noise.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f653f2c and ba567ee.

📒 Files selected for processing (3)
  • packages/jumpstarter-driver-composite/jumpstarter_driver_composite/driver.py (2 hunks)
  • packages/jumpstarter-driver-composite/jumpstarter_driver_composite/driver_test.py (2 hunks)
  • packages/jumpstarter/jumpstarter/driver/base.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/jumpstarter-driver-composite/jumpstarter_driver_composite/driver.py
🧰 Additional context used
🧬 Code graph analysis (2)
packages/jumpstarter/jumpstarter/driver/base.py (1)
packages/jumpstarter-driver-composite/jumpstarter_driver_composite/driver.py (1)
  • report (42-45)
packages/jumpstarter-driver-composite/jumpstarter_driver_composite/driver_test.py (2)
packages/jumpstarter-driver-composite/jumpstarter_driver_composite/driver.py (5)
  • Composite (17-18)
  • Proxy (22-53)
  • client (12-13)
  • client (27-28)
  • enumerate (47-48)
packages/jumpstarter/jumpstarter/driver/base.py (3)
  • Driver (56-273)
  • client (101-104)
  • enumerate (215-226)
⏰ 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: pytest-matrix (macos-15, 3.11)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
  • GitHub Check: Header rules - jumpstarter-docs
  • 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: e2e
  • GitHub Check: Pages changed - jumpstarter-docs
🔇 Additional comments (5)
packages/jumpstarter-driver-composite/jumpstarter_driver_composite/driver_test.py (4)

9-26: LGTM!

The MockSerial test double is well-structured with proper exports and a state flag to verify method invocation.


28-39: LGTM!

The MockParent test double correctly simulates a parent driver accessing child methods through the children dictionary, which is the intended usage pattern being tested.


63-84: LGTM!

The test properly verifies that Proxy forwards method calls to the target driver after enumerate() resolves the reference. The assertions check both return values and side effects.


87-111: LGTM!

The test comprehensively validates the parent-to-child delegation scenario through a Proxy, which is critical for the RideSX use case. The test properly simulates enumerate() before exercising the delegation chain.

packages/jumpstarter/jumpstarter/driver/base.py (1)

198-198: All calls to report() have been updated: no root parameter usage remains.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ae62043 and f653f2c.

📒 Files selected for processing (2)
  • packages/jumpstarter-driver-composite/jumpstarter_driver_composite/driver.py (2 hunks)
  • packages/jumpstarter-driver-composite/jumpstarter_driver_composite/driver_test.py (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/jumpstarter-driver-composite/jumpstarter_driver_composite/driver_test.py (3)
packages/jumpstarter-driver-composite/jumpstarter_driver_composite/driver.py (5)
  • Composite (17-18)
  • Proxy (22-51)
  • client (12-13)
  • client (27-28)
  • enumerate (45-46)
packages/jumpstarter/jumpstarter/common/utils.py (1)
  • serve (31-39)
packages/jumpstarter/jumpstarter/driver/base.py (3)
  • Driver (56-277)
  • client (101-104)
  • enumerate (219-230)
packages/jumpstarter-driver-composite/jumpstarter_driver_composite/driver.py (2)
packages/jumpstarter/jumpstarter/driver/base.py (4)
  • Driver (56-277)
  • client (101-104)
  • report (198-217)
  • enumerate (219-230)
packages/jumpstarter-driver-composite/jumpstarter_driver_composite/driver_test.py (2)
  • client (15-16)
  • client (32-33)
⏰ 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). (7)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
  • GitHub Check: pytest-matrix (macos-15, 3.12)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
  • GitHub Check: pytest-matrix (macos-15, 3.13)
  • GitHub Check: pytest-matrix (macos-15, 3.11)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
  • GitHub Check: e2e

When using a ref: that is internally used by another driver on the server side, forward the calls to the target driver.
Needs to resolve the target device first, and for that we need the actual root device as a reference point to start the search from.
Raise on improper use - trying to access report() or other driver call before the target is resolved.
Also change report() to not require "root", as the function always returns a report of itself - or the proxy target in case of Proxy driver
if not path:
raise ConfigurationError(f"Proxy driver {name} has empty path")
return reduce(lambda instance, name: instance.children[name], path, root)
self._proxy_target = reduce(lambda instance, name: instance.children[name], path, root)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use functional programming so little that I had to unshelve my textbooc to remember reduce ! (so sad! X-D)

@mangelajo mangelajo merged commit 923d9b6 into jumpstarter-dev:main Oct 14, 2025
18 checks passed
@jumpstarter-backport-bot
Copy link
Copy Markdown

Successfully created backport PR for release-0.7:

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants