hterik commented on code in PR #29809:
URL: https://github.com/apache/airflow/pull/29809#discussion_r1135729101
##########
airflow/kubernetes/kube_client.py:
##########
@@ -105,16 +107,26 @@ def get_kube_client(
if conf.getboolean("kubernetes_executor", "enable_tcp_keepalive"):
_enable_tcp_keepalive()
+ new_client_config = Configuration.get_default_copy()
+ api_client_retry_configuration = conf.getjson("kubernetes",
"api_client_retry_configuration", fallback={})
+
+ if not conf.getboolean("kubernetes_executor", "verify_ssl"):
+ _disable_verify_ssl()
+
+ if isinstance(api_client_retry_configuration, dict) and
api_client_retry_configuration != {}:
+ new_client_config.retries =
urllib3.util.Retry(**api_client_retry_configuration)
+ else:
+ raise ValueError("api_client_retry_configuration should be a
dictionary")
Review Comment:
Put a breakpoint where it is raising the exception or add the actual
type+value as part of the exception message, to help with the debugging and see
what you actually get back.
I suspect it's the `and api_client_retry_configuration != {}` that should be
exempt from the exception-condition. Because that is the value you get as
fallback when the config is not provided. Missing value is still a valid
configuration and should be allowed with fallback, not the same as
value-present-but-invalid :)
--
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]
