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/08/24 18:00:25 UTC
[GitHub] [airflow] vandonr-amz opened a new pull request, #33703: always push ECS task ARN to xcom in `EcsRunTaskOperator`
vandonr-amz opened a new pull request, #33703:
URL: https://github.com/apache/airflow/pull/33703
that xcom value was removed in #29447 and it broke our system test that relied on that xcom value
While I agree that changing the logic around how to reattach is good, removing the xcom value is potentially a breaking change.
It used to be written only when the task didn't complete, but I think returning it all the time makes sense.
--
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] Taragolis commented on pull request #33703: always push ECS task ARN to xcom in `EcsRunTaskOperator`
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on PR #33703:
URL: https://github.com/apache/airflow/pull/33703#issuecomment-1692213788
I have no objection for returning this value, just my 50 cents
---
This never work as it described
https://github.com/apache/airflow/blob/03fcbcc96efb3f34c2d4e219dbaccb7771197670/airflow/providers/amazon/aws/operators/ecs.py#L652-L656
Airflow should clear all previous XComs for the task instance before `execute` happen
---
Initial solution might work but it save value in the "fake task_id", which might be a reason for some issues
https://github.com/apache/airflow/blob/fc0250f1d5c43784f353dbdf4a34089aa96c28e5/airflow/providers/amazon/aws/operators/ecs.py#L280-L289
--
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] eladkal merged pull request #33703: always push ECS task ARN to xcom in `EcsRunTaskOperator`
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal merged PR #33703:
URL: https://github.com/apache/airflow/pull/33703
--
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 pull request #33703: always push ECS task ARN to xcom in `EcsRunTaskOperator`
Posted by "vandonr-amz (via GitHub)" <gi...@apache.org>.
vandonr-amz commented on PR #33703:
URL: https://github.com/apache/airflow/pull/33703#issuecomment-1692244407
> Oh, miss that this operator never `return` value as it might be expected.
yeah, the "return value" is the last line of the logs, only in some case... I don't think it's a good idea, but we have to live with it.
--
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 pull request #33703: always push ECS task ARN to xcom in `EcsRunTaskOperator`
Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas commented on PR #33703:
URL: https://github.com/apache/airflow/pull/33703#issuecomment-1692185433
> It used to be written only when the task didn't complete, but I think returning it all the time makes sense.
We'll now have to support this new/expanded behaviour (unless/until we deprecate it of course). But on the grand scheme of things I think this is fine :+1:
--
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] vincbeck commented on pull request #33703: always push ECS task ARN to xcom in `EcsRunTaskOperator`
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on PR #33703:
URL: https://github.com/apache/airflow/pull/33703#issuecomment-1692213800
Unit tests are failing
--
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