Repository navigation
SageMakerProcessingOperator does not honor action_if_job_exists #21711
Description
Activity
Thanks for opening your first issue here! Be sure to follow the issue template!
Feel free to submit PR
It looks like this is still an issue. I am packing for a vacation, but perhaps someone else can pick it up while I'm away. The full error message is
Traceback (most recent call last): File "/opt/airflow/airflow/providers/amazon/aws/operators/sagemaker.py", line 185, in execute raise AirflowException( airflow.exceptions.AirflowException: A SageMaker processing job with name processing-job already exists.so it may be as simple as adding an
if action_if_job_exists=='increment'section to the except block of this checkI looked at the issue and I agree, this change 96dd703 made a breaking change by stop using the parameter
action_if_job_exists. However, I dont see any valid solution to fix it. If we still want to increment the job name ifaction_if_job_existsisincrement, then we need to uselist_processing_jobsand then having back the issue #16763. The only partial valid solution I can see is to generate a random number as increment but then we might run into other issues:- What range should we take for this number (0..??)
- The implementation would deviate from the function signature since we do not increment the job name but suffix the name with a random number
My honest opinion here is we should deprecate the parameter
action_if_job_existsand users should handle it themselves and apply whatever strategy they wantReacted by D. Ferruzzi and Jarek PotiukMy honest opinion here is we should deprecate the parameter action_if_job_exists and users should handle it themselves and apply whatever strategy they want
I'm all for it. To my prespective it goes beyond what Airflow can/should do.
I think this is one of the cases where a blog post showing how to customize strategy is more suitable.Reacted by D. Ferruzzi and Jarek PotiukA fairly common recipe is to handle the
ThrottlingExceptionin situations like this. So thelist_processing_jobs(or more specifically the_list_requesthelper`) can catch that exception when it's exhausted the quota, and then sleep for a second or two and then continue another burst of requests.This way we don't drop existing functionality and the code remains backwards compatible. We've been a bit heavy-handed with deprecations and breaking changes in the Amazon Provider package as of late.
Reacted by Shubham Mehta, Vincent and D. FerruzziI am okay with that too. I still feel like this feature is odd and should have never been there but I agree, as a compromise we can go with what @o-nikolas described as solution
Reacted by D. FerruzziSounds like a plan. I'll try to get it implemented in the next few days.
Apache Airflow Provider(s)
amazon
Versions of Apache Airflow Providers
apache-airflow-providers-amazon | 2.4.0
Apache Airflow version
2.2.3 (latest released)
Operating System
Amazon Linux 2
Deployment
MWAA
Deployment details
No response
What happened
Sagemaker Processing Operator no longer honors the
action_if_job_existsparam and always fails creation of a new processing job is a job with the name already exists.This happens because in a recent change, the function responsible for executing the job no longer honors the increment setting:
Change that breaks the increment: 96dd703
New code:
airflow/airflow/providers/amazon/aws/operators/sagemaker.py
Line 167 in 6734eb1
What you expected to happen
When Sagemaker Processing operator is called with a job-name that already exists, the job creation should succeed with a name that is incremented by 1.
How to reproduce
invoke SageMakerProcessingOperator twice with the same job name while keeping action_if_job_exists as 'increment'.
Anything else
No response
Are you willing to submit PR?
Code of Conduct