Skip to content

Commit

Permalink
Increase ConflictException retries to 4 total (#36337)
Browse files Browse the repository at this point in the history
* Increase ConflictException retries to 4 total

We have seen a recent uptick in ConflictExceptions we receive from
SageMaker. It's not many, but enough to fail our system tests
unnecessarily.

Bumping the retires to 4 which I still think is very reasonable,
considering it's a very tight loop of 0.3 seconds sleep per retry.

* Update airflow/providers/amazon/aws/hooks/sagemaker.py

Co-authored-by: Andrey Anshin <Andrey.Anshin@taragol.is>

* Fix tests

---------

Co-authored-by: Andrey Anshin <Andrey.Anshin@taragol.is>
  • Loading branch information
o-nikolas and Taragolis committed Dec 21, 2023
1 parent 30afa46 commit 0b32613
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 3 deletions.
2 changes: 1 addition & 1 deletion airflow/providers/amazon/aws/hooks/sagemaker.py
Expand Up @@ -1138,7 +1138,7 @@ def stop_pipeline(
if check_interval is None:
check_interval = 10

for retries in (2, 1, 0):
for retries in reversed(range(5)):
try:
self.conn.stop_pipeline_execution(PipelineExecutionArn=pipeline_exec_arn)
except ClientError as ce:
Expand Down
6 changes: 4 additions & 2 deletions tests/providers/amazon/aws/hooks/test_sagemaker.py
Expand Up @@ -811,6 +811,8 @@ def test_stop_pipeline_retries_on_conflict(self, mock_conn):
operation_name="empty",
)
mock_conn().stop_pipeline_execution.side_effect = [
conflict_error,
conflict_error,
conflict_error,
conflict_error,
None,
Expand All @@ -819,7 +821,7 @@ def test_stop_pipeline_retries_on_conflict(self, mock_conn):
hook = SageMakerHook(aws_conn_id="aws_default")
hook.stop_pipeline(pipeline_exec_arn="test")

assert mock_conn().stop_pipeline_execution.call_count == 3
assert mock_conn().stop_pipeline_execution.call_count == 5

@patch("airflow.providers.amazon.aws.hooks.sagemaker.SageMakerHook.conn", new_callable=mock.PropertyMock)
def test_stop_pipeline_fails_if_all_retries_error(self, mock_conn):
Expand All @@ -833,7 +835,7 @@ def test_stop_pipeline_fails_if_all_retries_error(self, mock_conn):
with pytest.raises(ClientError) as raised_exception:
hook.stop_pipeline(pipeline_exec_arn="test")

assert mock_conn().stop_pipeline_execution.call_count == 3
assert mock_conn().stop_pipeline_execution.call_count == 5
assert raised_exception.value == conflict_error

@patch("airflow.providers.amazon.aws.hooks.sagemaker.SageMakerHook.conn", new_callable=mock.PropertyMock)
Expand Down

0 comments on commit 0b32613

Please sign in to comment.