hterik commented on code in PR #26710:
URL: https://github.com/apache/airflow/pull/26710#discussion_r981342716
##########
airflow/kubernetes/kube_client.py:
##########
@@ -19,25 +19,18 @@
import logging
+import urllib3
+
from airflow.configuration import conf
log = logging.getLogger(__name__)
try:
from kubernetes import client, config
- from kubernetes.client import Configuration
+ from kubernetes.client import ApiClient, Configuration
from kubernetes.client.rest import ApiException
has_kubernetes = True
-
- def _disable_verify_ssl() -> None:
- if hasattr(Configuration, 'get_default_copy'):
- configuration = Configuration.get_default_copy()
- else:
- configuration = Configuration()
- configuration.verify_ssl = False
- Configuration.set_default(configuration)
Review Comment:
Is it ok to remove this `set_default` and only rely on every other code path
going through `get_kube_client`?
I see in `pod_generator` and `TaskInstance` creates `ApiClient` in many
places, but only uses it for offline operations.
It's also created by `hooks.kubernetes`, haven't looked into what that does
yet.
-----
Maybe it's safer to keep it this way and incorporate the new config using
this same method?
##########
airflow/kubernetes/kube_client.py:
##########
@@ -104,16 +97,25 @@ def get_kube_client(
if conf.getboolean('kubernetes', 'enable_tcp_keepalive'):
_enable_tcp_keepalive()
+ client_config = Configuration.get_default_copy()
+
+ retryparams = conf.getjson('kubernetes',
'client_retry_configuration_kwargs', fallback={})
+ if retryparams != {}:
+ client_config.retries = urllib3.util.Retry(**retryparams)
Review Comment:
Is this level of configuration granularity good? Or is it enough to only
expose the backoff and number?
I could even go as far as saying some kind of backoff should be enabled by
default, without configuration.
--
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]
