[GitHub] [airflow] dstandish commented on a diff in pull request #28436: Log FileTaskHandler to work with multi_namespace_mode when running using KubernetesExecutor

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

dstandish commented on code in PR #28436:
URL: https://github.com/apache/airflow/pull/28436#discussion_r1051559929


##########
airflow/utils/log/file_task_handler.py:
##########
@@ -191,16 +191,20 @@ def _read(self, ti: TaskInstance, try_number: int, metadata: dict[str, Any] | No
                 log += f"*** {str(e)}\n"
                 return log, {"end_of_log": True}
         elif self._should_check_k8s(ti.queue):
+            pod_override = ti.executor_config.get("pod_override")
+            if pod_override and pod_override.metadata and pod_override.metadata.namespace:
+                namespace = pod_override.metadata.namespace
+            else:
+                namespace = conf.get("kubernetes_executor", "namespace")

Review Comment:
   looks good.  i think ultimately it would probably be better to store the actual namespace on the TI record somewhere.  then we don't need to check two places, and... maybe there ultimately will be other ways to configure namespace.  i thought about suggesting storing it in executor config (e.g. maybe simply under a key `namespace`) but... probably that should be left for user configuration so i dunno.  any thoughts?
   
   



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