[GitHub] [airflow] vandonr-amz commented on a diff in pull request #29447: Remove non-public interface usage in EcsRunTaskOperator

["vandonr-amz (via GitHub)" <gi...@apache.org>]

vandonr-amz commented on code in PR #29447:
URL: https://github.com/apache/airflow/pull/29447#discussion_r1304671155


##########
airflow/providers/amazon/aws/operators/ecs.py:
##########
@@ -650,27 +642,17 @@ def _start_task(self, context):
         self.arn = response["tasks"][0]["taskArn"]
         self.log.info("ECS task ID is: %s", self._get_ecs_task_id(self.arn))
 
-        if self.reattach:
-            # Save the task ARN in XCom to be able to reattach it if needed
-            self.xcom_push(context, key=self.REATTACH_XCOM_KEY, value=self.arn)

Review Comment:
   this is a somewhat-breaking change, as the example code
   https://github.com/apache/airflow/blob/e0f21f43c63b13fd48f55aa660746edc37df1458/tests/system/providers/amazon/aws/example_ecs_fargate.py#L125-L126
   was recommending setting reattach to true to get the ARN.
   
   I think this sucked, but removing the arn entirely from the xcom values is not good either.
   What we could do is set it all the time now that we d'ont rely on this anymore to know if we need to reattach.



-- 
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