Update functional.py #419
Update functional.py #419
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR adds error handling to the infra/modal/functional.py file, enhancing code robustness and debugging capabilities for the Infinity project's Modal deployment.
- Added try-except blocks in
BaseInfinityModelmethodsdownload_modelandenterto catch and log potential errors during model downloading and engine array initialization. - Implemented error handling in
InfinityModalmethodsembed,image_embed,rerank, andclassify, returningNoneif exceptions occur during processing. - Enclosed the
mainfunction's operations in a try-except block to catch and print any errors that may occur during remote method calls. - Consider adding more specific error types and logging mechanisms for better error tracking and debugging.
1 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
| print(f"Error embedding sentences: {e}") | ||
| return None |
There was a problem hiding this comment.
logic: returning None silently may lead to unexpected behavior in calling code. Consider raising a custom exception
| classifications_1 if classifications_1 else "N/A", | ||
| ) | ||
| except Exception as e: | ||
| print(f"Error in main entrypoint: {e}") |
There was a problem hiding this comment.
style: consider using a more specific exception type instead of catching all exceptions
|
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 #419 +/- ##
=======================================
Coverage 79.13% 79.13%
=======================================
Files 40 40
Lines 3173 3173
=======================================
Hits 2511 2511
Misses 662 662 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
@YadlaMani thanks for the improvement. I have not used this in a long time? (2months)
How does it improve the debugging?
Would it be helpful to raise an speicifc error instead of printing and returning None?
Have you tried it out with modal?
There was a problem hiding this comment.
PR Summary
(updates since last review)
This PR modifies error handling in the Modal functional API example, but has some implementation concerns that need to be addressed.
- Returning
Nonesilently inInfinityModalmethods masks failures and makes debugging harder - should raise specific exceptions instead - Generic
Exceptioncatches in try-except blocks are too broad - should catch specific error types likeModelLoadErrororInferenceError - Error messages only use
print()statements - should utilize proper logging with levels (error, warning, etc.) - Missing error propagation mechanism to inform callers of failures in
BaseInfinityModelmethods - No validation that the code works with Modal deployment - needs testing to verify error handling in production environment
The changes add basic error visibility but need refinement to be truly useful for debugging and production use.
1 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
| try: | ||
| print(f"downloading models {self.model_id} ...") | ||
| self._get_array() | ||
| except Exception as e: | ||
| print(f"Error downloading model: {e}") |
There was a problem hiding this comment.
style: consider propagating the error since this is a critical initialization step that should not fail silently
| try: | ||
| print("Starting the engine array ...") | ||
| self.engine_array = self._get_array() | ||
| await self.engine_array.astart() | ||
| print("engine array started!") | ||
| except Exception as e: | ||
| print(f"Error starting the engine array: {e}") |
There was a problem hiding this comment.
logic: initialization failure here could leave the engine in an invalid state - consider cleanup or proper error propagation
This reverts commit d1fa1c1.
Incorporate error handling to enhance code quality and simplify debugging.