shahar1 commented on code in PR #68557:
URL: https://github.com/apache/airflow/pull/68557#discussion_r3508523261
##########
providers/google/src/airflow/providers/google/cloud/hooks/kubernetes_engine.py:
##########
@@ -76,7 +76,7 @@ def get_conn(self) -> client.ApiClient:
configuration = self._get_config()
configuration.refresh_api_key_hook = self._refresh_api_key_hook
if self.enable_tcp_keepalive:
- _enable_tcp_keepalive()
+ _enable_tcp_keepalive(configuration)
Review Comment:
You should append `# use next version` to the `cncf.kubernetes` provider
version in `pyproject.toml`, otherwise it will use the old version (please note
my comment regarding making it a public function).
##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/kube_client.py:
##########
@@ -51,26 +51,34 @@ def _disable_verify_ssl() -> None:
_import_err = e
-def _enable_tcp_keepalive() -> None:
+def _enable_tcp_keepalive(configuration: Configuration) -> None:
Review Comment:
I'm concerned that it had been defined as a private function, yet imported
by another module. Now as you break the interface by making `configuration`
mandatory.
I'd suggest move the logic into a public method `def
enable_tcp_keepalive(...)` and keep it as a thin wrapper until deprecation.
--
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]
