-
Notifications
You must be signed in to change notification settings - Fork 114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revert "Update functional.py" #466
Conversation
This reverts commit d1fa1c1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This PR reverts error handling improvements that were previously added to the Modal deployment functionality in functional.py, removing defensive programming practices.
- Removed try-catch blocks from critical operations in
infra/modal/functional.py
including model downloads and engine startup - Eliminated null checks in print statements that safely handled failed operations
- Reverted error handling for embedding, image embedding, reranking and classification methods
- Increased risk of unhandled exceptions that could crash the Modal deployment
- Removed debugging capabilities that helped identify failure points in the deployment pipeline
1 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile
print(f"downloading models {self.model_id} ...") | ||
self._get_array() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: model download failures will now crash the application instead of being caught and logged
print("Starting the engine array ...") | ||
self.engine_array = self._get_array() | ||
await self.engine_array.astart() | ||
print("engine array started!") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: engine array startup failures will now crash the application instead of being caught and logged
"Success, all tasks submitted! Embeddings:", | ||
embeddings_1[0].shape, | ||
embeddings_2[0].shape, | ||
"Rerankings:", | ||
rerankings_1, | ||
"Classifications:", | ||
classifications_1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: accessing shape of potentially failed operations may cause NullPointerException
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #466 +/- ##
=======================================
Coverage 79.04% 79.04%
=======================================
Files 42 42
Lines 3408 3408
=======================================
Hits 2694 2694
Misses 714 714 ☔ View full report in Codecov by Sentry. |
Reverts #419