Miretpl commented on code in PR #71349:
URL: https://github.com/apache/airflow/pull/71349#discussion_r3790223761


##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/hooks/kubernetes.py:
##########
@@ -1024,14 +1025,29 @@ async def _get_field(self, field_name):
 
     @contextlib.asynccontextmanager
     async def get_conn(self) -> AsyncGenerator[async_client.ApiClient, None]:
-        kube_client = None
+        await self._load_config()
+        if self._config_loaded:
+            # Reuse one client per hook: each construction runs 
ssl.create_default_context()
+            # on the event loop and opens a new connection pool. Owners 
release it via
+            # close(); triggers do so in cleanup().
+            if self._cached_kube_client is None:
+                # No await between check and assign, or the loser's client 
leaks.

Review Comment:
   ```suggestion
   ```



##########
providers/google/src/airflow/providers/google/cloud/hooks/kubernetes_engine.py:
##########
@@ -514,13 +517,22 @@ def __init__(
 
     @contextlib.asynccontextmanager
     async def get_conn(self) -> AsyncGenerator[async_client.ApiClient, None]:
-        kube_client = None
-        try:
-            kube_client = await self._load_config()
-            yield kube_client
-        finally:
-            if kube_client is not None:
-                await kube_client.close()
+        # Reuse one client per hook; see AsyncKubernetesHook.get_conn for the 
rationale.

Review Comment:
   IMHO it should be rather copied here



-- 
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]

Reply via email to