henry3260 commented on PR #62616: URL: https://github.com/apache/airflow/pull/62616#issuecomment-4015781496
> Thanks for doing this. You are way out of date on your working branch, that file no longer exists. But the same change would be in `airflow-core/src/airflow/executors/workloads/callback.py` > > As far as the change itself, I think it should be alright. Just to think this through, there is currently a PR up that will be adding TestConnection workloads to the executor, and likely in the near future we'll be migrating on_success_callback, on_failure_callback, etc over to the ExecutorCallback framework. I think this still makes sense for the on_foo_callback; those are either be assigned to a Dag or a task, so either way it'll have this info, so those should be fine. > > And the TestConnection doesn't go through the callback flow so that should be fine too. > > Update your branch and make the change in the right/new location and ping me for a re-review, I'll approve it. Thanks for your information! -- 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]
