fix: auto-detect image compression in StorageMux flashing - #974
Conversation
|
Warning Review limit reached
Next review available in: 37 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughStorageMux storage writes now use ChangesStorageMux decompression
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant StorageMuxFlasherClient
participant MockStorageMux
participant AutoDecompressIterator
participant StorageFile
StorageMuxFlasherClient->>MockStorageMux: flash image resource
MockStorageMux->>AutoDecompressIterator: inspect and iterate resource
AutoDecompressIterator-->>MockStorageMux: decompressed or unchanged chunks
MockStorageMux->>StorageFile: write image chunks
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@python/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver_test.py`:
- Around line 183-196: Extend
test_driver_mock_storage_mux_flasher_auto_decompress’s compression parameter
matrix with a zstd-compressed input and matching test ID, using the project’s
existing zstd dependency or helper. Preserve the same flash/dump round-trip
assertion so StorageMuxFlasherClient zstd decompression is covered, then run
make pkg-test-jumpstarter-driver-opendal.
- Around line 230-231: Update the test server cleanup around server and
server_thread so the HTTP server is managed with a context manager or
contextlib.closing, and in finally call server.shutdown() followed by
server_thread.join(). Ensure the listening socket and serving thread are both
closed before the test exits.
🪄 Autofix
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 Plus
Run ID: c65a073f-c44d-4dbf-b279-5bfd2b12c1bc
📒 Files selected for processing (4)
python/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/client.pypython/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver.pypython/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver_test.pypython/packages/jumpstarter/jumpstarter/common/storage.py
d75ef71 to
c6add4f
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
c6add4f to
9ea6fad
Compare
| @@ -786,7 +786,11 @@ def flash( | |||
| operator: Operator | None = None, | |||
| compression: Compression | None = None, | |||
| ): | |||
There was a problem hiding this comment.
Not super familiar with compression, but how likely is it, that those magic bytes might appear randomly in some files? If its somewhat likely, we might want to check extensions and magic bytes.
| [gzip.compress, lambda data: lzma.compress(data, format=lzma.FORMAT_XZ), bz2.compress, zstd.compress], | ||
| ids=["gzip", "xz", "bz2", "zstd"], | ||
| ) | ||
| def test_driver_mock_storage_mux_flasher_auto_decompress(tmp_path, compress): |
There was a problem hiding this comment.
There are no tests for the explicit compression argument override in flash().
|
|
||
| if sys.version_info >= (3, 14): | ||
| from compression import zstd | ||
| else: |
There was a problem hiding this comment.
import here but is not declared as a dev dependency in this package's own pyproject.toml
2 participants
Fixes #54
Flashing a compressed image through the StorageMux drivers (SDWire, DUTLink) wrote the compressed bytes to the device unless the user passed
--compression xz.