potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports URL: https://github.com/apache/airflow/pull/6596#discussion_r352747261
########## File path: airflow/executors/local_executor.py ########## @@ -43,40 +42,44 @@ This option could lead to the unification of the executor implementations, running locally, into just one `LocalExecutor` with multiple modes. """ - -import multiprocessing import subprocess -from queue import Empty - -from airflow.executors.base_executor import BaseExecutor +from multiprocessing import Manager, Process +from multiprocessing.managers import SyncManager +from queue import Empty, Queue # pylint: disable=unused-import # noqa: F401 +from typing import Any, List, Optional, Tuple, Union # pylint: disable=unused-import # noqa: F401 + +from airflow import AirflowException Review comment: I don't think it matters that much any more. for sure it does not matter for the "python" cyclic imports (as you explained) but it matters for Pylint ones (those are detected differently - using different algorithms). I think that Pylint cycles are slightly less forgiving but still valid and they are working in a slightly different way (still have not figured all the details). But I think it does not matter any more for most of the imports from airflow (now that we removed operators/hooks/ etc. It still matters for 'models' because we are importing all models there so I prefer to keep 'from airflow.models.baseoperator import BaseOperator' in the core rather than 'from airflow.models import BaseOperator'. We might revisit that in the future however. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
