Skip to content
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

SagemakerProcessingOperator stopped honoring existing_jobs_found #27456

Merged
merged 3 commits into from
Nov 11, 2022

Conversation

ferruzzi
Copy link
Contributor

@ferruzzi ferruzzi commented Nov 2, 2022

Closes: #21711

The increment-name functionality was broken by #19195

I added back the code which increments the job name if the user says to, and added in a check for ThrottleException with a (user-overrideable) max of 3 retries and a bit of backoff (2/4/8 seconds) with an eventual exception after that. Also added a few unit tests to catch future regressions.

@ferruzzi ferruzzi requested a review from eladkal as a code owner November 2, 2022 00:38
@boring-cyborg boring-cyborg bot added area:providers provider:amazon AWS/Amazon - related issues labels Nov 2, 2022
eladkal
eladkal previously requested changes Nov 4, 2022
airflow/providers/amazon/aws/hooks/sagemaker.py Outdated Show resolved Hide resolved
@ferruzzi ferruzzi force-pushed the ferruzzi/sagemaker-increment branch from b8c1420 to 996fbd3 Compare November 8, 2022 08:08
@ferruzzi
Copy link
Contributor Author

ferruzzi commented Nov 8, 2022

Added the method back with deprecation and rebased

@ferruzzi ferruzzi force-pushed the ferruzzi/sagemaker-increment branch from 996fbd3 to 57c2c3e Compare November 8, 2022 08:11
@potiuk potiuk requested a review from eladkal November 8, 2022 13:42
@potiuk
Copy link
Member

potiuk commented Nov 8, 2022

Docs failure @ferruzzi :(

@ferruzzi
Copy link
Contributor Author

ferruzzi commented Nov 8, 2022

I think that should do it. I'll rebase again once the tests are passing.

@ferruzzi ferruzzi force-pushed the ferruzzi/sagemaker-increment branch from dd5cee5 to 89e3848 Compare November 8, 2022 18:03
@ferruzzi
Copy link
Contributor Author

ferruzzi commented Nov 8, 2022

Tests all passing; rebased

@potiuk potiuk dismissed eladkal’s stale review November 11, 2022 00:28

Points addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:amazon AWS/Amazon - related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SageMakerProcessingOperator does not honor action_if_job_exists
4 participants