o-nikolas commented on code in PR #73014:
URL: https://github.com/apache/airflow/pull/73014#discussion_r3994478520
##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/kube_client.py:
##########
@@ -177,6 +178,11 @@ def get_kube_client(
:param config_file: configuration file
:return: kubernetes client
"""
+ # An import path rather than a callable, so that KubernetesJobWatcher can
re-resolve it in
+ # its own process, where the spawn start method would not carry a callable
over.
+ client_factory = conf.get("kubernetes_executor", "client_factory",
fallback=None)
+ if client_factory:
+ return import_string(client_factory)()
Review Comment:
Looks like there is a conf getter for imports, might simplify things (one
less import, error handling, etc)
```suggestion
if client_factory := conf.getimport("kubernetes_executor",
"client_factory", fallback=None):
return client_factory()
```
##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/kube_client.py:
##########
Review Comment:
It looks like there are more callers of this than just the executor (one
example:
`providers/apache/spark/src/airflow/providers/apache/spark/hooks/spark_submit.py`).
Are we sure we want that big of a blast radius?
--
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]
