ridesx: fix cleanup of decompressed files#248
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAfter decompression, the RideSX driver computes the decompressed file’s path relative to the storage root and registers that path with the Opendal storage backend. Opendal gains a new public Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
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/driver.py`:
- Around line 115-117: The code directly mutates Opendal's private attributes
(_created_paths and _normalize_path) from RideSX driver; add a public method on
Opendal, e.g. register_path(self, path: str), that performs normalization and
insertion into the internal created paths set, then replace the direct access in
jumpstarter_driver_ridesx.driver (the lines using
storage._created_paths.add(storage._normalize_path(...))) with a call to
storage.register_path(str(relative_path)); ensure register_path encapsulates any
locking/normalization logic so external callers never touch private members.
57f9280 to
18588de
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
python/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver.py (1)
242-242:@validate_call(validate_return=True)missing — inconsistent with all other exported path-taking methodsEvery
@exportmethod that accepts apath: strparameter (e.g.open,stat,copy,rename,remove_all,create_dir,delete,exists,presign_stat, etc.) is guarded by@validate_call(validate_return=True). Omitting it here means an invalidpathvalue arriving over RPC won't be caught at the boundary.♻️ Proposed fix
`@export` + `@validate_call`(validate_return=True) async def register_path(self, path: str) -> None:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver.py` at line 242, Add the missing RPC input/output validation to the register_path method by decorating register_path with `@validate_call`(validate_return=True) (the same pattern used on other exported path-taking methods like open, stat, copy, etc.); ensure the validate_call symbol is imported where other methods use it so the decorator is available and applied to the register_path function to validate the incoming path and returned value at the RPC boundary.
🤖 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-opendal/jumpstarter_driver_opendal/driver.py`:
- Around line 241-252: Add the missing RPC argument/return validation to
register_path by decorating the method with `@validate_call`(validate_return=True)
so its path parameter is validated consistently with other exported methods
(open, stat, copy, rename, delete); update the register_path definition (which
calls self._normalize_path and adds to self._created_paths) to include the
`@validate_call`(validate_return=True) decorator above it.
---
Nitpick comments:
In
`@python/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver.py`:
- Line 242: Add the missing RPC input/output validation to the register_path
method by decorating register_path with `@validate_call`(validate_return=True)
(the same pattern used on other exported path-taking methods like open, stat,
copy, etc.); ensure the validate_call symbol is imported where other methods use
it so the decorator is available and applied to the register_path function to
validate the incoming path and returned value at the RPC boundary.
currently decompressed files were not tracked for cleanup by opendal, and thus are left on the storage consuming space Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
18588de to
a0eb230
Compare
currently decompressed files were not tracked for cleanup by opendal, and thus are left on the storage consuming space
Summary by CodeRabbit