You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "ferruzzi (via GitHub)" <gi...@apache.org> on 2024/09/23 20:55:19 UTC
[PR] Add STOPPED to the failure cases for Sagemaker Training Jobs [airflow]
ferruzzi opened a new pull request, #42423:
URL: https://github.com/apache/airflow/pull/42423
User pointed out that Airflow will mark a failed Sagemaker Training Job as a successful task. Looking into the issue, Sagemaker treats "Stopped" as a failure state for this endpoint, not a successful terminal state. [Sagemaker Docs [here](https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/sagemaker/client/describe_training_job.html)]
To reproduce:
Run a DAG with a SagemakerTrainingOperator task. Once the training starts, open the AWS console and stop the job. Airflow will see that the job has ended but mark the task as successful.
After this change, those steps will show the task marked as failed as expected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add STOPPED to the failure cases for Sagemaker Training Jobs [airflow]
Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas commented on code in PR #42423:
URL: https://github.com/apache/airflow/pull/42423#discussion_r1773759746
##########
airflow/providers/amazon/aws/hooks/sagemaker.py:
##########
@@ -155,6 +155,7 @@ class SageMakerHook(AwsBaseHook):
endpoint_non_terminal_states = {"Creating", "Updating", "SystemUpdating", "RollingBack", "Deleting"}
pipeline_non_terminal_states = {"Executing", "Stopping"}
failed_states = {"Failed"}
+ training_failed_states = {*failed_states, "Stopped"}
Review Comment:
Got it, thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add STOPPED to the failure cases for Sagemaker Training Jobs [airflow]
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #42423:
URL: https://github.com/apache/airflow/pull/42423#discussion_r1772067521
##########
airflow/providers/amazon/aws/hooks/sagemaker.py:
##########
@@ -155,6 +155,7 @@ class SageMakerHook(AwsBaseHook):
endpoint_non_terminal_states = {"Creating", "Updating", "SystemUpdating", "RollingBack", "Deleting"}
pipeline_non_terminal_states = {"Executing", "Stopping"}
failed_states = {"Failed"}
+ training_failed_states = {*failed_states, "Stopped"}
Review Comment:
Sagemaker has so many endpoints, and more popping up so often that I am not willing to guarantee that none of them do or will ever use "Stopped" as a successful end state, so I am only adding it to the Training failed states.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add STOPPED to the failure cases for Sagemaker Training Jobs [airflow]
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi merged PR #42423:
URL: https://github.com/apache/airflow/pull/42423
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add STOPPED to the failure cases for Sagemaker Training Jobs [airflow]
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #42423:
URL: https://github.com/apache/airflow/pull/42423#discussion_r1772301752
##########
airflow/providers/amazon/aws/hooks/sagemaker.py:
##########
@@ -155,6 +155,7 @@ class SageMakerHook(AwsBaseHook):
endpoint_non_terminal_states = {"Creating", "Updating", "SystemUpdating", "RollingBack", "Deleting"}
pipeline_non_terminal_states = {"Executing", "Stopping"}
failed_states = {"Failed"}
+ training_failed_states = {*failed_states, "Stopped"}
Review Comment:
`'TrainingJobStatus': 'InProgress'|'Completed'|'Failed'|'Stopping'|'Stopped',
`
"Completed" is the terminal successful state.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add STOPPED to the failure cases for Sagemaker Training Jobs [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #42423:
URL: https://github.com/apache/airflow/pull/42423#issuecomment-2369376188
Oh, I have so little knowledge about AWS and Sagemaker, feel afraid to judge and approve this thing :-(
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add STOPPED to the failure cases for Sagemaker Training Jobs [airflow]
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on PR #42423:
URL: https://github.com/apache/airflow/pull/42423#issuecomment-2369387977
> Oh, I have so little knowledge about AWS and Sagemaker, feel afraid to judge and approve this thing :-(
It's a very large ball of wax, that's for sure. I'm not entirely sure why you were auto-added to the reviewers list, I didn't intentionally tag you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add STOPPED to the failure cases for Sagemaker Training Jobs [airflow]
Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas commented on code in PR #42423:
URL: https://github.com/apache/airflow/pull/42423#discussion_r1772300230
##########
airflow/providers/amazon/aws/hooks/sagemaker.py:
##########
@@ -155,6 +155,7 @@ class SageMakerHook(AwsBaseHook):
endpoint_non_terminal_states = {"Creating", "Updating", "SystemUpdating", "RollingBack", "Deleting"}
pipeline_non_terminal_states = {"Executing", "Stopping"}
failed_states = {"Failed"}
+ training_failed_states = {*failed_states, "Stopped"}
Review Comment:
"Stopped" is really a failed state? What is the state called when the training job completes successfully? Is it not Stopped?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org