You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/11/07 23:38:25 UTC
[GitHub] [airflow] vandonr-amz opened a new pull request, #27551: Sagemaker hook: remove extra call at the end when waiting for completion
vandonr-amz opened a new pull request, #27551:
URL: https://github.com/apache/airflow/pull/27551
also took the opportunity to speedup tests relying on this method by waiting zero
--
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
[GitHub] [airflow] vandonr-amz commented on a diff in pull request #27551: Sagemaker hook: remove extra call at the end when waiting for completion
Posted by GitBox <gi...@apache.org>.
vandonr-amz commented on code in PR #27551:
URL: https://github.com/apache/airflow/pull/27551#discussion_r1016003750
##########
airflow/providers/amazon/aws/hooks/sagemaker.py:
##########
@@ -688,19 +687,16 @@ def check_status(
except ClientError:
raise AirflowException("AWS request failed, check logs for more info")
- if status in non_terminal_states:
- running = True
- elif status in self.failed_states:
+ if status in self.failed_states:
raise AirflowException(f"SageMaker job failed because {response['FailureReason']}")
- else:
- running = False
+ elif status not in non_terminal_states:
+ break
if max_ingestion_time and sec > max_ingestion_time:
# ensure that the job gets killed if the max ingestion time is exceeded
raise AirflowException(f"SageMaker job took more than {max_ingestion_time} seconds")
self.log.info("SageMaker Job completed")
- response = describe_function(job_name)
Review Comment:
there is basically no latency between the last call of the loop (l.682) and this one, and moreover the status reached is supposed to be a terminal state, so there is no need to repeat the call here.
##########
airflow/providers/amazon/aws/hooks/sagemaker.py:
##########
@@ -673,9 +673,8 @@ def check_status(
non_terminal_states = self.non_terminal_states
sec = 0
- running = True
- while running:
+ while True:
Review Comment:
had to do this change otherwise python would complain that the `response` variable might be uninitialized in the return clause.
--
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
[GitHub] [airflow] potiuk merged pull request #27551: Sagemaker hook: remove extra call at the end when waiting for completion
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #27551:
URL: https://github.com/apache/airflow/pull/27551
--
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