chore: stop logging "SyntaxError" as exceptions #21787
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
betodealmeida
approved these changes
Oct 12, 2022
| @@ -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.
Can we keep the debug? Or does it also show up as exceptions?
There was a problem hiding this comment.
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.
If we need the exception info in general it's better to use logger.exception, as it does that automatically:
Suggested change
| 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 |
mistercrunch
added
🏷️ bot
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
Moving exception logs to upper level
command.pyfile 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