fix(crewai-files): replace bare raises with PermanentUploadError in get_uploader - #6841
fix(crewai-files): replace bare raises with PermanentUploadError in get_uploader#6841jpcj223 wants to merge 2 commits into
Conversation
…et_uploader Bare raise statements outside of except blocks cause RuntimeError with a confusing 'No active exception to re-raise' message. - Replace bare raise for unsupported provider with PermanentUploadError - Replace bare raise for unconfigured bedrock with PermanentUploadError - Add unit tests for factory function and error paths
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe uploader factory now raises explicit ChangesUploader factory behavior
🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@lib/crewai-files/src/crewai_files/uploaders/factory.py`:
- Around line 200-204: Update the Bedrock uploader configuration validation in
get_uploader to reject bucket_name when it is None or otherwise empty, not
merely when the key is absent, so the actionable PermanentUploadError is raised
for missing bucket configuration. Add a regression test covering
get_uploader("bedrock", bucket_name=None).
🪄 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: 120bff8f-fa24-4e3b-8b4e-3bf585b43e7f
📒 Files selected for processing (2)
lib/crewai-files/src/crewai_files/uploaders/factory.pylib/crewai-files/tests/test_uploader_factory.py
Problem
Bare
raisestatements inget_uploader()are used outside ofexceptblocks, which causes a confusingRuntimeError: No active exception to re-raiseinstead of a meaningful error message.Two affected locations:
Solution
Replace both bare
raisestatements withPermanentUploadErrorso users get a clear, actionable error message.Changes
lib/crewai-files/src/crewai_files/uploaders/factory.py:PermanentUploadErrorfromcrewai_files.processing.exceptionsPermanentUploadErrorlisting supported providersPermanentUploadErrorexplaining how to configurelib/crewai-files/tests/test_uploader_factory.py:get_uploader()covering all supported providers and error pathsTesting
All 7 new tests pass: