dabla commented on code in PR #69128:
URL: https://github.com/apache/airflow/pull/69128#discussion_r3507565832
##########
providers/microsoft/azure/src/airflow/providers/microsoft/azure/hooks/msgraph.py:
##########
@@ -394,7 +398,21 @@ def get_conn(self) -> RequestAdapter:
@staticmethod
def _is_http_client_closed(request_adapter: RequestAdapter) -> bool:
"""Return True when the underlying httpx AsyncClient has been
closed."""
- return cast("HttpxRequestAdapter",
request_adapter)._http_client.is_closed
+ adapter = cast("HttpxRequestAdapter", request_adapter)
+
+ if adapter._http_client.is_closed:
+ return True
+
+ provider = cast("BaseBearerTokenAuthenticationProvider",
adapter._authentication_provider)
+ access_token_provider = cast("AzureIdentityAccessTokenProvider",
provider.access_token_provider)
+ credential = cast(
+ "ClientSecretCredential | CertificateCredential",
access_token_provider._credentials
+ )
+ transport = credential._client._pipeline._transport
+
+ if transport.session is not None:
+ return transport.session.closed
+ return False
Review Comment:
No the `httpx` client of the `RequestAdapter` is different from the pipeline
`aiohttp` session in the `CredentialsProvider`. If the session is None then it
would mean the `CredentialsProvider` hasn't done any calls yet, even though the
`httpx` client of the `RequestAdapter` could already be initialised for example.
I don't know if this scenario could really occur, but the previous PR wasn't
enough, as sometimes `httpx` client of the `RequestAdapter` could still be
open/valid but the `CredentialsProvider` session could be closed, and thus
re-using the cached `RequestAdapter` instance would fail anyway on the next
refresh token invocation, thus beter to discard it.
I could have catch the `RuntimeError`, but that would mean you would try to
use the invalid `RequestAdapter`, then see it fails because session of
`CredentialsProvider` is closed and then retry the request again with a new
`RequestAdapter`, which I don't like as this means catching an exception which
you could have detected upfront (exceptions are as the are saying exceptions,
thus something you can't check upfront) and that would also mean doing 2
requests while with above solution we just do it in one request.
--
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]
