You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "vandonr-amz (via GitHub)" <gi...@apache.org> on 2023/07/13 17:24:25 UTC
[GitHub] [airflow] vandonr-amz opened a new pull request, #32589: BUGFIX: AWS ECS trigger returned "success" after max_attempts instead of failing
vandonr-amz opened a new pull request, #32589:
URL: https://github.com/apache/airflow/pull/32589
reported by user in issue #32580
the issue is about something else, but the user mentioned this as a "bonus bug"
--
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 #32589: BUGFIX: AWS ECS trigger returned "success" after max_attempts instead of failing
Posted by "vandonr-amz (via GitHub)" <gi...@apache.org>.
vandonr-amz commented on code in PR #32589:
URL: https://github.com/apache/airflow/pull/32589#discussion_r1264020600
##########
airflow/providers/amazon/aws/triggers/ecs.py:
##########
@@ -179,8 +181,7 @@ async def run(self) -> AsyncIterator[TriggerEvent]:
finally:
if self.log_group and self.log_stream:
logs_token = await self._forward_logs(logs_client, logs_token)
-
- yield TriggerEvent({"status": "success", "task_arn": self.task_arn})
+ raise AirflowException("Waiter error: max attempts reached")
Review Comment:
I disagree, yielding an event will send this back to the scheduler, and require a worker to be setup and everything just to throw an error there. Might as well save all this by raising directly in the triggerer.
I was discussed quickly in this slack thread: https://apache-airflow.slack.com/archives/CCPRP7943/p1685663968275419
--
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] uranusjr merged pull request #32589: BUGFIX: AWS ECS trigger returned "success" after max_attempts instead of failing
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr merged PR #32589:
URL: https://github.com/apache/airflow/pull/32589
--
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] o-nikolas commented on a diff in pull request #32589: BUGFIX: AWS ECS trigger returned "success" after max_attempts instead of failing
Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas commented on code in PR #32589:
URL: https://github.com/apache/airflow/pull/32589#discussion_r1265677100
##########
airflow/providers/amazon/aws/triggers/ecs.py:
##########
@@ -179,8 +181,7 @@ async def run(self) -> AsyncIterator[TriggerEvent]:
finally:
if self.log_group and self.log_stream:
logs_token = await self._forward_logs(logs_client, logs_token)
-
- yield TriggerEvent({"status": "success", "task_arn": self.task_arn})
+ raise AirflowException("Waiter error: max attempts reached")
Review Comment:
Sound good @uranusjr ?
--
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] uranusjr commented on a diff in pull request #32589: BUGFIX: AWS ECS trigger returned "success" after max_attempts instead of failing
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #32589:
URL: https://github.com/apache/airflow/pull/32589#discussion_r1263374934
##########
airflow/providers/amazon/aws/triggers/ecs.py:
##########
@@ -179,8 +181,7 @@ async def run(self) -> AsyncIterator[TriggerEvent]:
finally:
if self.log_group and self.log_stream:
logs_token = await self._forward_logs(logs_client, logs_token)
-
- yield TriggerEvent({"status": "success", "task_arn": self.task_arn})
+ raise AirflowException("Waiter error: max attempts reached")
Review Comment:
This should yield a failure event instead
--
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