Skip to content

Conversation

vandonr-amz
Copy link
Contributor

reported by user in issue #32580
the issue is about something else, but the user mentioned this as a "bonus bug"

… of failing

reported by user in issue apache#32580
the issue is about something else, but the user mentionned this as a "bonus bug"
@boring-cyborg boring-cyborg bot added area:providers provider:amazon AWS/Amazon - related issues labels Jul 13, 2023
@@ -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")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should yield a failure event instead

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sound good @uranusjr ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:amazon AWS/Amazon - related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants