o-nikolas commented on code in PR #27185:
URL: https://github.com/apache/airflow/pull/27185#discussion_r1008320041
##########
airflow/providers/microsoft/azure/hooks/batch.py:
##########
@@ -359,18 +359,44 @@ def wait_for_job_tasks_to_complete(self, job_id: str,
timeout: int) -> list[batc
incomplete_tasks = [task for task in tasks if task.state !=
batch_models.TaskState.completed]
if not incomplete_tasks:
# detect if any task in job has failed
- fail_tasks = [
+ failed_tasks = [
task
for task in tasks
- if task.executionInfo.result
- ==
batch_models.TaskExecutionInformation.TaskExecutionResult.failure
+ if (task.execution_info.result ==
batch_models.TaskExecutionResult.failure) or (
+ task.execution_info.exit_code != 0)
]
- return fail_tasks
+ return failed_tasks
for task in incomplete_tasks:
self.log.info("Waiting for %s to complete, currently on %s
state", task.id, task.state)
time.sleep(15)
raise TimeoutError("Timed out waiting for tasks to complete")
+ def wait_for_single_job_task_to_complete(self, job_id: str, task_id: str,
+ timeout: int) ->
batch_models.CloudTask | None:
+ """
+ Wait for a single task in a particular job to complete
+
+ :param job_id: A string that identifies the job
+ :param task_id: A string that identifies the task
+ :param timeout: The amount of time to wait before timing out in minutes
Review Comment:
Something about the return value, name and doc string don't quite match up
with the behaviour of the code itself. A task is only returned in a failure
case, and nothing (well none) is returned if it is successful. The code calling
this is really just using the returned task as a boolean to detect whether it
was a pass or fail. What about:
```suggestion
def wait_for_single_job_task_to_complete(self, job_id: str, task_id: str,
timeout: int) -> bool:
"""
Wait for a single task in a particular job to complete, return False
if it ultimately fails or True if it succeeds.
:param job_id: A string that identifies the job
:param task_id: A string that identifies the task
:param timeout: The amount of time to wait before timing out in
minutes
:return: A bool that represents whether or not the task failed
(false) or succeeded (true)
```
Just a minor nit though
--
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]
