potiuk commented on code in PR #53126:
URL: https://github.com/apache/airflow/pull/53126#discussion_r2222943763


##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/hooks/kubernetes.py:
##########
@@ -93,6 +93,7 @@ class KubernetesHook(BaseHook, PodOperatorHookProtocol):
     :param cluster_context: Optionally specify a context to use (e.g. if you 
have multiple
         in your kubeconfig.
     :param config_file: Path to kubeconfig file.
+    :param config_dict: Takes the config file as a dict.

Review Comment:
   Yeah. I think dict and file are quite different cases, But also what we miss 
here (and maybe in other places it is already use) is to explain what happens 
when you specify none or both. This also might give even better justification 
for having two parameters if we implement the case where specifying both 
**merges** the configuration (and we need to specify what is the merge 
direction),
   
   I think it makes sense (if it is possible) to handle "both" in the way that 
dict config overrides corresponding configs from file. 
   
   If we do it this way, then having two parameters makes even more sense. If 
we make the parameters mutually exclusive - we should 
   
   a) definitely at least write about it
   b) potentially replace it with single parameter accepting either string 
(File) or dict (config).
   
   I would favour separate parameters and merging as it allows some interesting 
cases like overriding production/development parameters from the same file 
using configuration or the other way round - using common configuration in dict 
but having different content of config on devel/prod.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to