ChickenBenny commented on code in PR #75561: URL: https://github.com/apache/airflow/pull/75561#discussion_r4386126821
########## task-sdk/src/airflow/sdk/coordinators/_subprocess.py: ########## Review Comment: Thanks for picking this up! One small catch with the new contract though: it doesn't quite hold on the native Dag path. `_build_native_task_command` wraps `_build_dag_file_command` in a bare `except Exception`: https://github.com/apache/airflow/blob/c385eff4dbcff65b1b0baf17bf757bac4fcac518/task-sdk/src/airflow/sdk/coordinators/_subprocess.py#L859-L864 So if a subclass does what the docstring now says and raises `TaskLaunchError(..., retryable=False)`, it gets caught there and re-wrapped into a fresh `TaskLaunchError`. Should be a two-line fix, same as the guard does: ```python except TaskLaunchError: raise except Exception as e: ... ``` plus a test where _build_dag_file_command raises a TaskLaunchError and we check it comes out untouched. I've got that test locally already. Want me to push it, or would you rather fold it in yourself? -- 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]
