[sqllab] More detailed async error messages for sqllab #8164
Conversation
serenajiang
force-pushed
the
serena-verbose-sqllab-errors
branch
3 times, most recently
from
0e22a97 to
9cda5f9
Compare
Codecov Report
@@ Coverage Diff @@
## master #8164 +/- ##
======================================
Coverage 66.2% 66.2%
======================================
Files 479 479
Lines 22930 22930
Branches 2524 2524
======================================
Hits 15180 15180
Misses 7616 7616
Partials 134 134
Continue to review full report at Codecov.
|
serenajiang
changed the title
superset/db_engine_specs/hive.py
Outdated
| @@ -200,7 +200,7 @@ def extract_error_message(cls, e): | |||
| match = re.search(r'errorMessage="(.*?)(?<!\\)"', msg) | |||
| if match: | |||
| msg = match.group(1) | |||
| return msg | |||
| return f"Hive error: {msg}" | |||
There was a problem hiding this comment.
I would much prefer to see this in base.py along the lines of f"{self.engine} error: {msg}" for uniformity and only the engine-specific message extraction logic where necessary.
There was a problem hiding this comment.
I was thinking a similar thing, having each place that returns an error go through an error formatting function that prepends the engine
serenajiang
force-pushed
the
serena-verbose-sqllab-errors
branch
from
9cda5f9 to
4a4298f
Compare
serenajiang
changed the title
serenajiang
force-pushed
the
serena-verbose-sqllab-errors
branch
from
4a4298f to
86955b0
Compare
superset/db_engine_specs/base.py
Outdated
| @@ -476,7 +476,11 @@ def handle_cursor(cls, cursor, query, session): | |||
| pass | |||
|
|
|||
| @classmethod | |||
| def extract_error_message(cls, e: Exception) -> str: | |||
| def extract_error_message(cls, e: Exception): | |||
There was a problem hiding this comment.
pls add a return type here! -> str
There was a problem hiding this comment.
I added a return type for extract_error_message but left all the private _extract_error_messages without a return type. Mostly because I wanted to avoid putting in a # noqa (ugh dict.get). But also, since extract_error_message string formats the result of _extract_error_message, the return type of _extract_error_message shouldn't matter.
serenajiang
force-pushed
the
serena-verbose-sqllab-errors
branch
from
86955b0 to
5810df2
Compare
serenajiang
changed the title
mistercrunch
added
🏷️ bot
CATEGORY
Choose one
SUMMARY
Add source of error (presto, hive, etc.) into error messages for sqllab. This will hopefully confuse users a little less.
REVIEWERS
@etr2460 @graceguo-supercat