dylanschw commented on code in PR #62090: URL: https://github.com/apache/airflow/pull/62090#discussion_r2822752613
########## 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: Thanks for calling this out. As written, using airflow.sdk.execution_time.execute_workload is not compatible with Airflow 2.x since airflow.sdk is only available in Airflow 3.x. I will update the executor command construction to branch on Airflow version, using the SDK entrypoint for Airflow 3.x and the legacy task runner entrypoint for Airflow 2.x. I will add unit tests that assert the exact command built for both Airflow >= 3.0 and Airflow < 3.0, including the entrypoint and arguments like --json-string. Since the regression occurred there, I will take another look at test_try_adopt_task_instances for ECS and Batch. If unskipping is not feasible due to environment assumptions, I will add a targeted test to cover the same behavior with mocking. On factoring this into executor/utils, that sounds reasonable. I can do it in this PR -- 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]
