airflow-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports
Date Sat, 30 Nov 2019 22:20:19 GMT
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_r352307296
 
 

 ##########
 File path: airflow/kubernetes/pod_generator.py
 ##########
 @@ -26,35 +26,7 @@
 
 import kubernetes.client.models as k8s
 
-from airflow.executors import Executors
-
-
-class PodDefaults:
-    """
-    Static defaults for the PodGenerator
-    """
-    XCOM_MOUNT_PATH = '/airflow/xcom'
-    SIDECAR_CONTAINER_NAME = 'airflow-xcom-sidecar'
-    XCOM_CMD = 'trap "exit 0" INT; while true; do sleep 30; done;'
-    VOLUME_MOUNT = k8s.V1VolumeMount(
-        name='xcom',
-        mount_path=XCOM_MOUNT_PATH
-    )
-    VOLUME = k8s.V1Volume(
-        name='xcom',
-        empty_dir=k8s.V1EmptyDirVolumeSource()
-    )
-    SIDECAR_CONTAINER = k8s.V1Container(
-        name=SIDECAR_CONTAINER_NAME,
-        command=['sh', '-c', XCOM_CMD],
-        image='alpine',
-        volume_mounts=[VOLUME_MOUNT],
-        resources=k8s.V1ResourceRequirements(
-            requests={
-                "cpu": "1m",
-            }
-        ),
-    )
+from airflow.kubernetes.pod_defaults import PodDefaults
 
 Review comment:
   The problem here was more than PodDefaults. The podDefaults had originally dependency on
Executors
   ```
   from airflow.executors import Executors
   ```
   
   And that was the real problem.  Executors depended on specific executors (including Kubernetes
one) and then got to depend on PodLauncher which depended on PodGenerator which then back
depended on PodDefaults
   
   
   It is very often problem with dependencies that you create some common code (defaults)
that you use internally in your class (Pod Generator). Then you create another class to use
those common settings (from PodGenerator) and then in your PodGenerator you refer the other
class that uses defaults (PodLauncher in this case) and PodLauncher uses PodGenerator eventually
- causing cyclic dependency..
   
   This is classic 'cyclic dependency' problem. And the way to solve it is to move the common
code to a separate class (PodDefaults) that can be used from two places (PodGenerator and
 PodLauncher. This means that both PodGenerator and PodLauncher can use PodDefaults without
depending on each other.
   
   
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message