Skip to content

improve error message in ridesx flashing#543

Open
bennyz wants to merge 1 commit intojumpstarter-dev:mainfrom
bennyz:improve-ridesx-error
Open

improve error message in ridesx flashing#543
bennyz wants to merge 1 commit intojumpstarter-dev:mainfrom
bennyz:improve-ridesx-error

Conversation

@bennyz
Copy link
Copy Markdown
Member

@bennyz bennyz commented Apr 12, 2026

fixes #540

@netlify
Copy link
Copy Markdown

netlify bot commented Apr 12, 2026

Deploy Preview for jumpstarter-docs failed. Why did it fail? →

Name Link
🔨 Latest commit 9db6069
🔍 Latest deploy log https://app.netlify.com/projects/jumpstarter-docs/deploys/69db5c88fd8fd3000833d50e

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 12, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 593a7c4e-04b3-4e0d-9698-16038e9a6b21

📥 Commits

Reviewing files that changed from the base of the PR and between ab7546a and 9db6069.

📒 Files selected for processing (4)
  • python/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/client.py
  • python/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/client_test.py
  • python/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/driver.py
  • python/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/driver_test.py
✅ Files skipped from review due to trivial changes (3)
  • python/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/driver.py
  • python/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/driver_test.py
  • python/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/client.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • python/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/client_test.py

📝 Walkthrough

Walkthrough

Stricter OCI-path detection added; new detection for partition:filepath inputs; flash() and flash-command execution now produce targeted errors advising use of -t or per-partition partition:path when mappings/targets are misused. Tests added for these behaviors.

Changes

Cohort / File(s) Summary
Client changes
python/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/client.py
Tightened _is_oci_path heuristics, added _looks_like_partition_spec, improved flash() single-file error message for partition:path inputs, and made _execute_flash_command raise explicit -t/partition:path errors when mapping and positional non-OCI paths are combined.
Client tests
python/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/client_test.py
Added unit tests verifying _is_oci_path and partition-spec detection; tests for error messages from _execute_flash_command and flash() when partition:path or non-OCI inputs are used incorrectly with/without -t.
Driver changes
python/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/driver.py
Added RideSXDriver._validate_oci_url static method to centralize OCI validation and emit partition:path hint text; replaced inline oci:// checks in flash_oci_image with the new validator.
Driver tests
python/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/driver_test.py
Added tests for _validate_oci_url covering valid oci://, non-oci:// schemes, bare registry inputs, and various partition:path forms (absolute/relative/home/parent) including presence/absence of hint text.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • Add tests for BLE driver #536: Related changes to ridesx flash handling and confusing error messages when mixing -t flags and positional images.
  • jumpstarter-dev/jumpstarter#790: Overlapping work on partition:path vs OCI parsing and client-side flash validation/error messaging.

Suggested reviewers

  • mangelajo
  • bkhizgiy

Poem

🐰
I nibble through the messy paths,
I spot the colons, dodge the wraths,
If you forget the little -t,
I’ll twitch my nose and kindly plea:
“Pass partition:path with -t!” ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title directly describes the main change: improving error messages for RideSX flashing operations.
Description check ✅ Passed The description references issue #540, which is related to the changeset's goal of improving error messaging for multi-target flashing scenarios.
Linked Issues check ✅ Passed The code changes fully address the issue requirements by adding validation logic to detect partition:path misuse and provide targeted error messages guiding users to use -t flags correctly.
Out of Scope Changes check ✅ Passed All changes are focused on improving error handling and validation for RideSX flashing with multiple targets, directly addressing the linked issue requirements.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@python/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/client.py`:
- Around line 423-429: The current guard only checks
self._looks_like_partition_spec(path) and misses non-OCI positional inputs
(e.g., boot_a:boot.img, ./boot_a.simg) that should trigger the "missing -t"
error in multi-target mode; update the condition in the argument-parsing code
around that block to raise the ClickException whenever the positional path is
not an OCI reference (use whatever project helper detects OCI refs, e.g., an
existing is_oci_reference/_is_oci_reference function or a small url/regex check)
OR self._looks_like_partition_spec(path) or the value resembles a local
filesystem image (contains '/' or '\' or ends with common image extensions or
os.path.exists(path)); reference self._looks_like_partition_spec and the
OCI-detection helper when making the change so non-OCI inputs are caught and the
same explanatory ClickException is raised.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b77aa05a-ba91-41d6-b586-e27943c3b87e

📥 Commits

Reviewing files that changed from the base of the PR and between d7a65e0 and ab7546a.

📒 Files selected for processing (4)
  • python/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/client.py
  • python/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/client_test.py
  • python/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/driver.py
  • python/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/driver_test.py

Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
Assisted-by: claude-opus-4.6
@bennyz bennyz force-pushed the improve-ridesx-error branch from ab7546a to 9db6069 Compare April 12, 2026 08:49
@bennyz bennyz requested a review from mangelajo April 12, 2026 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

j storage flash calls can be confusing with multiple targets

1 participant