refactor: return initial exception and check if it's user error #21836
Conversation
Codecov Report
@@ Coverage Diff @@
## master #21836 +/- ##
==========================================
+ Coverage 66.96% 66.97% +0.01%
==========================================
Files 1807 1807
Lines 69222 69213 -9
Branches 7403 7403
==========================================
+ Hits 46352 46357 +5
+ Misses 20965 20951 -14
Partials 1905 1905
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 |
hughhhh
requested review from
AAfghahi and
pkdotson
and removed request for
betodealmeida
| except Exception as ex: | ||
| query_id = query.id if query else None | ||
| logger.exception("Query %d: %s", query_id, type(ex)) |
There was a problem hiding this comment.
would it be useful to still log query id here or no?
There was a problem hiding this comment.
I don't think having the id helps us debug the issue at all tbh, with that information we can't do anything else.
the important thing is that we log the exception here (
superset/superset/views/base.py
Line 202 in 9edb581
|
@betodealmeida @john-bodley Here are more examples of us refactoring to bubble up errors to the flask or celery level. |
mistercrunch
added
🏷️ bot
SUMMARY
Refactor sqllab to return initial exception and use the status code to indicate whether this error should be logged as an exception. We are going to log from the
handle_api_exceptiondecorator instead of the lower level.superset/superset/views/base.py
Line 202 in 9edb581
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION