[GitHub] KevinYang21 commented on issue #3873: [Airflow-2760] Decouple DAG parsing loop from scheduler loop
KevinYang21 commented on issue #3873: [Airflow-2760] Decouple DAG parsing loop from scheduler loop URL: https://github.com/apache/incubator-airflow/pull/3873#issuecomment-431199074 Is there more I can do to have this PR merged? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] KevinYang21 commented on issue #3873: [Airflow-2760] Decouple DAG parsing loop from scheduler loop
KevinYang21 commented on issue #3873: [Airflow-2760] Decouple DAG parsing loop from scheduler loop URL: https://github.com/apache/incubator-airflow/pull/3873#issuecomment-429241610 @Fokko Thanks for reviewing. I actually created [a ticket](https://issues.apache.org/jira/browse/AIRFLOW-3194) for further refactoring in the code base to use with block as there're a lot such cases and I feel it is beyond the scope of this PR to change all of them( updated all use cases that I touched). Would you actually elaborate a bit more on the "weird stuff in the number of connections" please? Might be something we didn't catch. Also this reminds me about one thing: with the new logic, it is more likely that the scheduler will leave orphan processes in extreme cases, e.g. for some reason the agent process crashed(or got a SIGTERM) or cannot gracefully exit, the dag parsing processes will be left behind. Should we put a note somewhere about this? Actually we already have similar problem, e.g. scheduler process got SIGTERM. Do we just expect people to know this and clean up the left over processes? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] KevinYang21 commented on issue #3873: [Airflow-2760] Decouple DAG parsing loop from scheduler loop
KevinYang21 commented on issue #3873: [Airflow-2760] Decouple DAG parsing loop from scheduler loop URL: https://github.com/apache/incubator-airflow/pull/3873#issuecomment-429124129 @aoen @Fokko @bolkedebruin @mistercrunch @kaxil any love for this old PR? quite a big PR and thus rebase experience is not very ideal :P This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] KevinYang21 commented on issue #3873: [Airflow-2760] Decouple DAG parsing loop from scheduler loop
KevinYang21 commented on issue #3873: [Airflow-2760] Decouple DAG parsing loop from scheduler loop URL: https://github.com/apache/incubator-airflow/pull/3873#issuecomment-426486693 @feng-tao Any chance you got some spare time to give this change a shot :D? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] KevinYang21 commented on issue #3873: [Airflow-2760] Decouple DAG parsing loop from scheduler loop
KevinYang21 commented on issue #3873: [Airflow-2760] Decouple DAG parsing loop from scheduler loop URL: https://github.com/apache/incubator-airflow/pull/3873#issuecomment-425585509 @ashb Thank you very much for the reviews! Totally understand your concern and I can definitely wait or try to work with other committers to have it tested to provide more confidence. Any volunteers :D ? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] KevinYang21 commented on issue #3873: [Airflow-2760] Decouple DAG parsing loop from scheduler loop
KevinYang21 commented on issue #3873: [Airflow-2760] Decouple DAG parsing loop from scheduler loop URL: https://github.com/apache/incubator-airflow/pull/3873#issuecomment-425301416 Updated docstring in DagFileProcessor[Agent|Manager] with more details and added docstring for method `execute_helper()` with details on how the scheduler loop works and link to the graphic representation. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] KevinYang21 commented on issue #3873: [Airflow-2760] Decouple DAG parsing loop from scheduler loop
KevinYang21 commented on issue #3873: [Airflow-2760] Decouple DAG parsing loop from scheduler loop URL: https://github.com/apache/incubator-airflow/pull/3873#issuecomment-424980424 @ashb Thanks for the feedback. I have a small updated the scheduler.rst to reflect the change but I didn't go into a lot details. The reason is that the document seems to me is facing Airflow users and they probly don't care too much about exactly what is happening, especially parts like how scheduler loop is divided into three logical pieces. From the existing doc, we stop at the details of DagFileProcessorManager( just mention that it would stay in sync with DAG folder but didn't go into the DagFileProcessor part), to me if we want to keep the same level of detail we should stop at briefly mention about DagFileProcessorManager and DagFileProcessorAgent level. Other the other hand, I can try add more comments in the code if you feel like it, as that would probly be the place where people care about implementation details are going to look at. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
