aaron-y-chen commented on code in PR #71349:
URL: https://github.com/apache/airflow/pull/71349#discussion_r3966392822
##########
providers/google/src/airflow/providers/google/cloud/hooks/kubernetes_engine.py:
##########
@@ -514,13 +517,21 @@ 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: construction parses the CA into an SSL
context on
+ # the event loop and opens a new connection pool. Released via
close()/cleanup().
+ if self._cached_kube_client is None:
+ self._cached_kube_client = self._build_client()
+ if self._cached_token is None:
+ self._cached_token = await self.get_token()
Review Comment:
I wonder whether caching this token could cause a long-running GKE trigger
to use expired credentials. The metadata server may return a cached token with
less than 3600 seconds remaining, but `_CredentialsToken.refresh()` always
records its lifetime as 3600 seconds, so `get()` may wait 1800 seconds before
refreshing it.
For example: the metadata server returns a token with 600 seconds remaining
-> this wrapper caches it -> 601 seconds later `get()` still considers it fresh
-> the next K8s API poll sends the expired token and receives 401.
Could we address this and add a regression test covering this case?
--
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]
