[GitHub] [airflow] XD-DENG commented on a diff in pull request #28047: Ensure KubernetesExecutor's multi_namespace_mode work without cluster-scope role

[GitBox <gi...@apache.org>]

XD-DENG commented on code in PR #28047:
URL: https://github.com/apache/airflow/pull/28047#discussion_r1038644918


##########
airflow/executors/kubernetes_executor.py:
##########
@@ -66,7 +67,7 @@ class ResourceVersion:
     """Singleton for tracking resourceVersion from Kubernetes"""
 
     _instance = None
-    resource_version = "0"
+    resource_version: dict[str | None, str | None] = {}

Review Comment:
   Hi @dstandish , when `multi_namespace_mode` is `True` & `namespace_list` is not specified, that means the user will still want to apply the way using cluster-scope role. In this case, argument `namespace` will be None for the corresponding `KubernetesJobWatcher`. That's they the key can be `None`.
   
   But the value doesn't have to be `str | None`. So I will change this to `resource_version: dict[str | None, str] = {}`.



##########
airflow/executors/kubernetes_executor.py:
##########
@@ -288,15 +294,33 @@ def _make_kube_watcher(self) -> KubernetesJobWatcher:
         watcher.start()
         return watcher
 
-    def _health_check_kube_watcher(self):
-        if self.kube_watcher.is_alive():
-            self.log.debug("KubeJobWatcher alive, continuing")
-        else:
-            self.log.error(
-                "Error while health checking kube watcher process. Process died for unknown reasons"
+    def _make_kube_watchers(self) -> dict[str | None, KubernetesJobWatcher]:

Review Comment:
   The same as clarified above.



-- 
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