Update ecs_task_id on reattaching#22879
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
|
|
Is it possible to add unit test to cover this change? |
Yep. We should |
|
I updated the test for the ECS operator to verify that the ID is always updated and matches the last part of the ARN. I am waiting to see how it will run on CI/CD (I don't have the setup to build or run all unit tests) |
| ) | ||
| if previous_task_arn in running_tasks: | ||
| self.arn = previous_task_arn | ||
| self.ecs_task_id = self.arn.split("/")[-1] |
There was a problem hiding this comment.
Small nit: When I had coded this (before I knew you were working on it 😆) I extracted setting the arn/task_id into a helper method, because this code is now duplicated in two spots. Not a huge deal, since ARNs are fairly stable, but worth considering. Approving
|
Needs rebase. |
@potiuk: could you please provide more details about the "rebase"? |
|
You will find it in the contributing doc (you got the link when you created your first PR). Just look for rebase there - it has all explanations and howtos.
|
|
@potiuk: I followed the instructions using my "providers-amazon" branch, and the upstream "providers-amazon" branch from the upstream airflow repo. When I asked for a rebase, it didn't do anything, since it didn't seem that anything was made to the upstream branch since the changes that I made. The instructions on the contributing document talk about the main branch, but I assumed that I should be using the 'providers-amazon" branch instead. I am probably missing something simple here, please let me know what I should do next. |
|
No. It's main branch. All changes in Airflow are done to main. You should not contribute anything to other branches. |
|
I deleted that branch - it was there by mistake. You should re-do your PR based on |
Resolve an issue that can occur when reattaching to ECS tasks.
closes: #22878
The code for fetching AWS logs, looks for the ecs_task_id field. When executing the task for the first time, this is updated after fetching the ARN for the first time. However, when the task is reattached to an existing ECS task, the current code will update the ARN, but will not update the ecs_task_id. The code change will ensure that the ecs_task_id is updated when the ARN is updated at the time of reattaching.