Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate whether the async credentials should be closed by this library or by the caller #365

Open
baywet opened this issue Dec 1, 2022 · 6 comments
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request

Comments

@baywet
Copy link
Member

baywet commented Dec 1, 2022

    [Their documentation](https://learn.microsoft.com/en-us/python/api/azure-core/azure.core.credentials_async.asynctokencredential?view=azure-python) is terrible but I'm wondering what close does and whether we need to call it after each authentication request or when we don't need the credential object anymore? (wondering whether we'll have the same issue as the http package where it stops working after the first request)

Originally posted by @baywet in microsoft/kiota-authentication-azure-python#45 (comment)

@baywet baywet added enhancement New feature or request dependencies Pull requests that update a dependency file labels Dec 1, 2022
@samwelkanda samwelkanda removed their assignment Jan 31, 2024
@sebastienlevert
Copy link

@baywet is this closed?

@baywet
Copy link
Member Author

baywet commented Feb 1, 2024

sure, it might be a problem in some scenarios (singleton instances of credentials etc...) but we'll wait for customers to report something before spending mort time on this first.

@baywet baywet closed this as not planned Won't fix, can't repro, duplicate, stale Feb 1, 2024
@dico-harigkev
Copy link

Hello, I would like to ask to reopen this issue, as it seems to manifest e.g. in microsoftgraph/msgraph-sdk-python#505 , preventing long living instances of msgraph.GraphServiceClient from refreshing access tokens using an async Credential once they expire after an hour.

My suggestion would be to leave closing the credential entirely up to the user. For them it would then be easy to control the lifecycle of their credential instance by using it as context manager.

Closing the credential inside the library means that there is no way for the user to keep client instances alive for longer than one token lifetime, or to reuse the same credential to instantiate multiple clients.

At least in the case of using azure.identity.aio credentials, it also makes the application crash with a very obscure exception (AttributeError: 'NoneType' object has no attribute '__aenter__') caused by the credential not expecting to be accessed after getting closed, which can not easily be caught and handled in user code. However, handling this status more robustly would need to be a change in that library instead of here, just adding it for context.

@Christiaan-Mathu
Copy link

Hi @baywet, hope you are well,

I have the same issue with GraphServiceClient and would like to know how I should be using it.
I am currently using it as a singleton and after the token expires then I get the following error:

venv/lib/python3.11/site-packages/azure/core/pipeline/transport/_aiohttp.py", line 255, in send
    result = await self.session.request(  # type: ignore
                   ^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'request'

What other approach should I be using if this is not correct?

@baywet baywet reopened this Feb 28, 2024
@baywet
Copy link
Member Author

baywet commented Feb 28, 2024

@samwelkanda can you follow up on this one when you have some time please?

@dico-harigkev
Copy link

@Christiaan-Mathu My current workaround is to prevent the credential from getting closed by the GraphServiceClient using a custom credential implementation.

I basically derive a custom class from the async credential class I want to use (here azure.identity.aio.DefaultAzureCredential) and override its async def close(self) method to do nothing.
Instead I add another custom method to the derived credential class, which calls the original super-class close method, so that I can still actually close the underlying credential from my own application code at the right time, just before terminating.

This seems to work fine, I had no issues with this approach so far.

@baywet baywet transferred this issue from microsoft/kiota-authentication-azure-python Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request
Projects
Status: Todo 📃
Development

No branches or pull requests

5 participants