vincbeck commented on PR #44369: URL: https://github.com/apache/airflow/pull/44369#issuecomment-2501831157
> There are `cleanup` function implementations that expect `task_instance` to be always present like #39442 and might fail to cleanup if they are used with asset based use cases though exceptions from cleanup are suppressed so just wondering if cleanup is semantically valid for triggers with assets scenario since it's not mentioned in the AIP. > > Edit : Just saw #42514 to handle cleanup so my message might not be valid. Please ignore if not needed. Thanks. I think it is fine here because when I read the examples you provided, you cancel external job (external from Airflow) if the task instance is not in deferred state. All the logic here implemented is specific to deferrable operators and should not overlap with this feature. At first I thought the triggers were being cleaned up but here it is external jobs, I dont see it overlapping. But thanks for heads-up! #42514 is being resolved as part of this PR, therefore the logic handling the trigger cleaned-up is done in that PR (at least on my perspective). So if you think something is missing or off, please call it out :) -- 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]
