-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
chore: stop logging "SyntaxError" as exceptions #21787
Conversation
@@ -313,8 +313,6 @@ def execute_sql_statement( # pylint: disable=too-many-arguments,too-many-statem | |||
if query.status == QueryStatus.STOPPED: | |||
raise SqlLabQueryStoppedException() from ex | |||
|
|||
logger.error("Query %d: %s", query.id, type(ex), exc_info=True) | |||
logger.debug("Query %d: %s", query.id, ex) |
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.
Can we keep the debug? Or does it also show up as exceptions?
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.
naw we end up eating it and converting to a payload
i'll add it back
superset/sqllab/command.py
Outdated
@@ -132,6 +132,7 @@ def run( # pylint: disable=too-many-statements,useless-suppression | |||
) from ex | |||
raise ex | |||
except Exception as ex: | |||
logger.error("Query %d: %s", query.id, type(ex), exc_info=True) |
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.
If we need the exception info in general it's better to use logger.exception
, as it does that automatically:
logger.error("Query %d: %s", query.id, type(ex), exc_info=True) | |
logger.exception("Query %d: %s", query.id, type(ex)) |
Codecov Report
@@ Coverage Diff @@
## master #21787 +/- ##
==========================================
- Coverage 66.86% 66.85% -0.01%
==========================================
Files 1803 1803
Lines 68996 68997 +1
Branches 7349 7349
==========================================
- Hits 46132 46131 -1
- Misses 20971 20973 +2
Partials 1893 1893
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
SUMMARY
Moving exception logs to upper level
command.py
file since we are doing filtering of Syntax errors there. The goals is not to log users errors on our end. This PR will be an example for other errors that we might not want to log as exceptions.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION