-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
Proper error handling in Hive Queries #4428
Conversation
Can we retest this ? Failed check has nothing to do with my patch. |
@john-bodley thanks |
ping @mistercrunch |
ping @john-bodley |
@betodealmeida @mistercrunch would you mind reviewing this PR given your experience with Hive? |
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.
Sorry about the delay. Curious as to in what kind of errors were mishandled.
superset/db_engine_specs.py
Outdated
state = cursor.poll() | ||
if state.operationState == ttypes.TOperationState.ERROR_STATE: | ||
raise Exception('Query error', state.errorMessage) | ||
if cls.limit_method == LimitMethod.FETCH_MANY: |
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.
Let's call super
here instead of repeating the code in the parent class
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.
Done.
Sorry for delay. I was on the vacations :)
Codecov Report
@@ Coverage Diff @@
## master #4428 +/- ##
=========================================
Coverage ? 77.48%
=========================================
Files ? 44
Lines ? 8716
Branches ? 0
=========================================
Hits ? 6754
Misses ? 1962
Partials ? 0
Continue to review full report at Codecov.
|
@mistercrunch |
I was trying to understand whether this conflicts with |
I'm using this code on production few months. So probable there is no conflict. |
* Proper error handling in Hive Queries * Change quotes * Trigger checks * Adding call to parent class * Small fix * Fix in method call
* Proper error handling in Hive Queries * Change quotes * Trigger checks * Adding call to parent class * Small fix * Fix in method call
* Proper error handling in Hive Queries * Change quotes * Trigger checks * Adding call to parent class * Small fix * Fix in method call
Resolves: #4414
Fetch_many gives strange exception when hive query is in Error state.
So we're reading status to get proper error message before calling fetch_many