SameerMesiah97 commented on code in PR #62090: URL: https://github.com/apache/airflow/pull/62090#discussion_r2819869700
########## providers/amazon/src/airflow/providers/amazon/aws/executors/ecs/ecs_executor.py: ########## @@ -69,6 +69,19 @@ class AwsEcsExecutor(BaseExecutor): + """Executor that runs Airflow tasks on AWS ECS.""" + + def _ti_to_execute_workload_cmd(self, ti): Review Comment: Maybe this functionality can be included in the executor/utils folder to avoid duplication.. I do not think this is a massive issue as it is only duplicated once but I think it should be considered. I would wait for the codeowner to weigh in here. ########## providers/amazon/src/airflow/providers/amazon/aws/executors/ecs/ecs_executor.py: ########## @@ -69,6 +69,19 @@ class AwsEcsExecutor(BaseExecutor): + """Executor that runs Airflow tasks on AWS ECS.""" + + def _ti_to_execute_workload_cmd(self, ti): + from airflow.executors.workloads import ExecuteTask + + workload_json = ExecuteTask.make(ti).model_dump_json() + return [ + "python3", + "-m", + "airflow.sdk.execution_time.execute_workload", + "--json-string", + workload_json, + ] Review Comment: Is this compatible with airflow 2.x? It seems like this will not work for versions before Airflow 3.0 as `airflow.sdk` was introduced then. I think you should have a look at PR #58207. There are issues with that but I think the implementation in that PR was more backwards compatible. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
