-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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
Refactor query polling logic for athena queries #21488
Conversation
f4a119d
to
7766ddd
Compare
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.
You'll need to wait for a committer to approve and merge, but LGTM. 👍 Thanks for adding this one.
Some tests are failing :( (related) |
@victorphoenix3 can you please fix the failing tests? |
7766ddd
to
2e456d6
Compare
@eladkal I have updated the PR. Please review |
@@ -72,7 +72,7 @@ def test_init(self): | |||
|
|||
assert self.athena.hook.sleep_time == 0 | |||
|
|||
@mock.patch.object(AthenaHook, 'check_query_status', side_effect=("SUCCESS",)) |
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.
'SUCCESS' is not a state for athena queries, "SUCCEEDED" is. The tests were failing because of this discrepancy
@victorphoenix3 can you please rebase? tests are broken due to issue with moto (was already fixed in #22046 ) |
2e456d6
to
2a5c0a8
Compare
@eladkal PR is ready |
Query status polling logic currently polls until the query is not in intermmediate state. It should instead poll until query is not in the terminal state.
Similar to #19877