Duo-ee commented on PR #72866: URL: https://github.com/apache/airflow/pull/72866#issuecomment-6544449578
Checked how this interacts with `AsyncKubernetesHook`, which lives in the same module (`providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/hooks/kubernetes.py:831` on `main`) and inherits `KubernetesHook.__init__`. `AsyncKubernetesHook.get_conn` (`hooks/kubernetes.py:1069-1074` on `main`) builds the client with `_TimeoutAsyncK8sApiClient(configuration=self.client_configuration)` and reads neither the hook argument nor the connection field. The result after this change: `AsyncKubernetesHook(ssl_ca_cert="/path/to/ca.crt")` accepts the argument without complaint but never applies it, and `configuration.ssl_ca_cert` stays unset unless the kubeconfig itself carries a `certificate-authority` entry. If you want to close that gap, plumbing the value into `_TimeoutAsyncK8sApiClient` the way you did for `_TimeoutK8sApiClient` would do it. Keeping the PR sync-only is also a valid choice — it just helps async users to know that the connection field applies to the sync hook only. I am not asking you to expand the scope, only flagging this because the new connection field looks like it works for both hooks. I used a coding agent to survey the code paths behind this comment. -- 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]
