[GitHub] [airflow] hussein-awala commented on a diff in pull request #29498: add missing read for K8S config file from conn in deferred `KubernetesPodOperator`

["hussein-awala (via GitHub)" <gi...@apache.org>]

hussein-awala commented on code in PR #29498:
URL: https://github.com/apache/airflow/pull/29498#discussion_r1105145411


##########
airflow/providers/cncf/kubernetes/operators/kubernetes_pod.py:
##########
@@ -565,7 +565,16 @@ def execute_async(self, context: Context):
 
     def convert_config_file_to_dict(self):
         """Converts passed config_file to dict format."""
-        config_file = self.config_file if self.config_file else os.environ.get(KUBE_CONFIG_ENV_VAR)
+        config_file = None

Review Comment:
   I am not sure about the initial reason to convert the file into dictionary before creating the trigger, it may be to avoid copying the config file to the triggerer, where the pod is created on the worker using the sync hook and the waiting task is running on the triggerer and it uses the async hook.
   
   > here is a risk that another is missing or new in the future would need "manual" fix like this
   
   With this fix, we cover all options currently available to provide the configuration file, and yes, if we add a new one in the future, we must add it on the sync hook and in this method.
   
   @VladaZakharova can you please explain what was the motivation to convert the config file to a dictionary before creating the trigger?



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