-
Notifications
You must be signed in to change notification settings - Fork 11
feat: Update error handling to log with warnings.warn()
#442
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
base: main
Are you sure you want to change the base?
Conversation
warnings.warn()warnings.warn()
src/galileo/utils/catch_log.py
Outdated
| except exception as err: | ||
| logger.warning(f"Error occurred during execution: {f.__name__}: {err}") | ||
| # Emit a warning that's visible by default (no logging config needed) | ||
| warnings.warn( |
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.
❌ Failed check: pre-commit / pre-commit
I’ve attached the relevant part of the log for your convenience:
pre-commit hook made changes to file: warnings.warn() call was reformatted from multi-line to single-line format in the warn_catch_exception function
Finding type: Log Error
| def wrapper(f: Callable) -> Callable: | ||
| @functools.wraps(f) | ||
| async def inner(*args: Any, **kwargs: Any) -> Any: | ||
| try: |
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.
❌ Failed check: pre-commit / pre-commit
I’ve attached the relevant part of the log for your convenience:
pre-commit hook made changes to file: warnings.warn() call was reformatted from multi-line to single-line format in the async_warn_catch_exception function
Finding type: Log Error
b3a8402 to
9896e4a
Compare
src/galileo/utils/catch_log.py
Outdated
| # Emit a warning that's visible by default (no logging config needed) | ||
| warnings.warn( | ||
| f"Galileo: {f.__name__} failed: {err}", | ||
| RuntimeWarning, |
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.
❌ Failed check: pre-commit / pre-commit
I’ve attached the relevant part of the log for your convenience:
pre-commit hook made changes to file: warnings.warn() call was reformatted from multi-line to single-line format (lines 355-359 collapsed to line 360)
Finding type: Log Error
9896e4a to
2fced79
Compare
| from typing import Any, Callable | ||
|
|
||
|
|
||
| def warn_catch_exception(logger: Logger = logging.getLogger(__name__), exception: type[Exception] = Exception) -> Any: |
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.
Removing support for the standard logger interface is not the correct approach.
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.
Sounds good, I added this back in addition to the warnings.warn() but sounds like we'll want to think about this a bit more deeply
2fced79 to
6c437bd
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #442 +/- ##
=======================================
Coverage 82.98% 82.98%
=======================================
Files 89 89
Lines 7980 8024 +44
=======================================
+ Hits 6622 6659 +37
- Misses 1358 1365 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
User description
Shortcut:
Description:
All SDK errors are caught by
catch_log.pywhich takes the logs and logs them to a specific catch_log logger which required the developer to configure their code like this if they wanted to see logs:There is little documentation on this, just this sdk method which isn't super helpful
This PR makes errors nosier by having them logged with
warnings.warn()as opposed to having the user have to configure this unique catch_log.Tests:
Generated description
Below is a concise technical summary of the changes proposed in this PR:
Enhance the
galileo-pythonSDK's error handling by modifying thewarn_catch_exceptionandasync_warn_catch_exceptiondecorators incatch_log.pyto emit visible warnings usingwarnings.warn(). This change ensures that exceptions caught within the SDK are more readily apparent to users without requiring specific logging configurations, improving the default observability experience.add_llm_spancalls in batch logger tests to include themodelparameter, ensuring test compatibility and completeness.Modified files (1)
Latest Contributors(2)
warn_catch_exceptionandasync_warn_catch_exceptiondecorators, ensuring users are notified without requiring specific logging configurations.Modified files (1)
Latest Contributors(2)