danmactough commented on a change in pull request #4039: [AIRFLOW-3046] Fix ECS
Operator mistakenly reports success
URL: https://github.com/apache/incubator-airflow/pull/4039#discussion_r224743337
##########
File path: airflow/contrib/operators/ecs_operator.py
##########
@@ -139,6 +140,15 @@ def _check_success_task(self):
raise AirflowException(response)
for task in response['tasks']:
+ # This is a `stoppedReason` that indicates a task has not
+ # successfully finished, but there is no other indication of
failure
+ # in the response.
+ # See,
https://docs.aws.amazon.com/AmazonECS/latest/developerguide/stopped-task-errors.html
# noqa E501
+ if re.match(r'Host EC2 \(instance .+?\) (stopped|terminated)\.',
Review comment:
> A look at the doc you linked to and it sounds like any of these other
stoppedReasons would also be "we had to kill your task" type things and should
be an error to?
Not quite, @ashb. "Essential container in task exited" is the stoppedReason
when your task successfully finishes. 🌀
I _think_ you're probably right about some of the other stoppedReasons
("Container instance deregistration forced by user" jumps out), but the problem
I'm trying to solve here is that when the host is terminated there is no other
evidence of failure -- and I don't have any idea if those other stoppedReasons
also have no evidence of failure or whether we can safely rely on the
container's exit status. The container's exit status seems like the most
precise indicator of success/failure, so I think it's preferable to use that
indicator -- but in this edge case, that indicator fails.
So, you certainly may be correct that there are more edge cases, but I think
we can limit this fix to the edge case we can prove.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
