-
Notifications
You must be signed in to change notification settings - Fork 23
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
Retry GetExecBatchResults on ApiExceptions caused by GSB Errors #588
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
da2f159
Implement re-tries for ApiExceptions caused by GSB Errors
azawlocki 6b04dd6
Endpoint -> endpoint
filipgolem c6356d9
Apply fixes after code review
azawlocki e5a2b3c
Fixes after code review: part II
azawlocki 02c638b
debug -> warning
filipgolem 25b2a14
Improve logs when activity is prematurely terminated on the provider
filipgolem d22fdd2
Formatting
filipgolem 3745a6b
Merge branch 'b0.6' into az/repeat-on-gsb-error
azawlocki a5d0824
Raise BatchError when an activity is terminated by the provider
azawlocki 3e9c236
Add unit tests for PollingBatch behavior when GSB errors occur
azawlocki File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,126 @@ | ||
from typing import List, Optional, Tuple, Type | ||
from unittest.mock import Mock | ||
|
||
import pytest | ||
|
||
from ya_activity.exceptions import ApiException | ||
from yapapi.rest.activity import BatchError, PollingBatch | ||
|
||
|
||
GetExecBatchResultsSpec = Tuple[Optional[Exception], List[str]] | ||
|
||
|
||
def mock_activity(specs: List[GetExecBatchResultsSpec]): | ||
"""Create a mock activity. | ||
|
||
The argument `specs` is a list of pairs specifying the behavior of subsequent calls | ||
to `get_exec_batch_results()`: i-th pair corresponds to the i-th call. | ||
The first element of the pair is an optional error raised by the call, the second element | ||
is the activity state (the `.state` component of the object returned by `Activity.state()`). | ||
""" | ||
i = -1 | ||
|
||
async def mock_results(*_args, **_kwargs): | ||
nonlocal specs, i | ||
i += 1 | ||
error = specs[i][0] | ||
if error: | ||
raise error | ||
return [Mock(index=0)] | ||
|
||
async def mock_state(): | ||
nonlocal specs, i | ||
state = specs[i][1] | ||
return Mock(state=state) | ||
|
||
return Mock(state=mock_state, _api=Mock(get_exec_batch_results=mock_results)) | ||
|
||
|
||
GSB_ERROR = ":( GSB error: some endpoint address not found :(" | ||
|
||
|
||
@pytest.mark.parametrize( | ||
"specs, expected_error", | ||
[ | ||
# No errors | ||
([(None, ["Running", "Running"])], None), | ||
# Exception other than ApiException should stop iteration over batch results | ||
( | ||
[(ValueError("!?"), ["Running", "Running"])], | ||
ValueError, | ||
), | ||
# ApiException not related to GSB should stop iteration over batch results | ||
( | ||
[(ApiException(status=400), ["Running", "Running"])], | ||
ApiException, | ||
), | ||
# As above, but with status 500 | ||
( | ||
[ | ||
( | ||
ApiException(http_resp=Mock(status=500, data='{"message": "???"}')), | ||
["Running", "Running"], | ||
) | ||
], | ||
ApiException, | ||
), | ||
# ApiException not related to GSB should raise BatchError if activity is terminated | ||
( | ||
[ | ||
( | ||
ApiException(http_resp=Mock(status=500, data='{"message": "???"}')), | ||
["Running", "Terminated"], | ||
) | ||
], | ||
BatchError, | ||
), | ||
# GSB-related ApiException should cause retrying if the activity is running | ||
( | ||
[ | ||
( | ||
ApiException(http_resp=Mock(status=500, data=f'{{"message": "{GSB_ERROR}"}}')), | ||
["Running", "Running"], | ||
), | ||
(None, ["Running", "Running"]), | ||
], | ||
None, | ||
), | ||
# As above, but max number of tries is reached | ||
( | ||
[ | ||
( | ||
ApiException(http_resp=Mock(status=500, data=f'{{"message": "{GSB_ERROR}"}}')), | ||
["Running", "Running"], | ||
) | ||
] | ||
* PollingBatch.GET_EXEC_BATCH_RESULTS_MAX_TRIES, | ||
ApiException, | ||
), | ||
# GSB-related ApiException should raise BatchError if activity is terminated | ||
( | ||
[ | ||
( | ||
ApiException(http_resp=Mock(status=500, data=f'{{"message": "{GSB_ERROR}"}}')), | ||
["Running", "Terminated"], | ||
) | ||
], | ||
BatchError, | ||
), | ||
], | ||
) | ||
@pytest.mark.asyncio | ||
async def test_polling_batch_on_gsb_error( | ||
specs: List[GetExecBatchResultsSpec], expected_error: Optional[Type[Exception]] | ||
) -> None: | ||
"""Test the behavior of PollingBatch when get_exec_batch_results() raises exceptions.""" | ||
|
||
PollingBatch.GET_EXEC_BATCH_RESULTS_INTERVAL = 0.1 | ||
|
||
activity = mock_activity(specs) | ||
batch = PollingBatch(activity, "batch_id", 1) | ||
try: | ||
async for _ in batch: | ||
pass | ||
assert expected_error is None | ||
except Exception as error: | ||
assert expected_error is not None and isinstance(error, expected_error) |
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
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.
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.
This conflicts with changes in #548 , which should be merged first.
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.
#548 was a PR to
master
, and the current PR is tob0.6
. When we will merge changes inb0.6
tomaster
we'll have to merge those two sets of changes, but no need to worry about this now. I don't think we plan to backport #548 tob0.6
, do we?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.
A similar PR will be a part of the next
yajsapi
release. IMO it would be great to backport #548.