jason810496 commented on PR #61436: URL: https://github.com/apache/airflow/pull/61436#issuecomment-3850928744
Thanks Jarek and Jed for sharing the context. I think there two parts we pointed out here 1. Whether the triggers are importable in Dag files 2. The refreshing mechanism for triggers > if we see a problem that it is not added, we should fix it. Agreed. For first part: Even though it's pointed out as bad practice as `This just formalizes that it's best practice to have triggers come from elsewhere on sys.path instead to avoid that problem.` in https://github.com/apache/airflow/pull/48603 From the user perspective, I would expect my triggers defined in Dag files could be imported by Triggerer, but the Triggerer doesn't respect `AIRFLOW__CORE__DAGS_FOLDER` currently. > In fact, triggers from Dag files was pretty broken before anyways, so we intentionally removed that ability in AF3. We are able to restore the importing triggers from Dag files by adding `sys.path.insert(0, conf.get("core", "dags_folder"))` before entering Triggerer's async loop. https://github.com/apache/airflow/blob/fe0633d729c85131ca96aa41a8c56282a407b7d5/airflow-core/src/airflow/jobs/triggerer_job_runner.py#L885-L890 > as the triggerer is unable to account for changes without having a restart. Then this level of problem will be second part: Unless we refactor Triggerer to run the triggers by `execution_time/task_runner` instead of running them as pure `asyncio` coroutines, we will not be able to make the Triggerer aware of the Dag Bundle. This will need much more discussion and effort and might not be finished soon. -- 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]
