dstandish commented on PR #20578:
URL: https://github.com/apache/airflow/pull/20578#issuecomment-1129105596

   > could we rename `config_file` -> `config_file_path` in the kubernetes hook 
?
   > 
   > cause currently I find very disturbing the variable name : `config_file`
   
   oy, yeah, you are right, could be named better.  i think i'm the one that 
added it a couple months back as prep work for this change, and i just kept the 
same name used in 
[KPO](https://github.com/apache/airflow/blob/fe6ec34f0d0e7d47d58913a71777fbfb6239b634/airflow/providers/cncf/kubernetes/operators/kubernetes_pod.py#L186).
  
   
   But honestly at this point I don't think I'm in favor of changing it.  
There's already been so much tumult and change with K8s /  KPO, and really it's 
not _that_ bad is it?  The risk is what, that a user would think it accepts a 
file handler?  It's annotated with Optional[str] (though maybe we should extend 
that to accept `Path`).  And the docstring should also clarify (though i guess 
i need to add that!)
   
   In any case, if we _do_ want to modify it, I think it belongs in a distinct 
PR for that.


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