You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/01/24 20:44:33 UTC

[GitHub] [airflow] eladkal commented on a change in pull request #20647: Set default value of in_cluster False for GKEStartPodOperator

eladkal commented on a change in pull request #20647:
URL: https://github.com/apache/airflow/pull/20647#discussion_r791136255



##########
File path: airflow/providers/google/cloud/operators/kubernetes_engine.py
##########
@@ -335,8 +339,16 @@ def __init__(
                 stacklevel=2,
             )
             is_delete_operator_pod = False
+        if in_cluster:
+            warnings.warn(
+                f"You should not set parameter `in_cluster` `True` in class {self.__class__.__name__}. "
+                "This operator uses Google Service Account(GSA) not Kubernetes Service Account(KSA)."
+                "If you can use KSA with in_cluster `True`, you should use KubernetesPodOperator directly.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
 
-        super().__init__(is_delete_operator_pod=is_delete_operator_pod, **kwargs)
+        super().__init__(is_delete_operator_pod=is_delete_operator_pod, in_cluster=in_cluster, **kwargs)

Review comment:
       Wouldn't it be easier to just set in_cluster=False directly?
   According to your description there is no other alternative in any case.

##########
File path: airflow/providers/google/cloud/operators/kubernetes_engine.py
##########
@@ -306,6 +306,9 @@ class GKEStartPodOperator(KubernetesPodOperator):
         pod; if False, leave the pod.  Current default is False, but this will be
         changed in the next major release of this provider.
     :type is_delete_operator_pod: bool
+    :param in_cluster: Run kubernetes client with in_cluster configuration. This parameter is
+        for backward compatibility. No need to change this parameter from default now.
+    :type in_cluster: bool

Review comment:
       If the user can't really sent the value of this parameter then why should we expose it in the constructor?




-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org