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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
