Change introduced in "Try anon=True if no credentials are supplied or found" broke connection to private S3 compatible storage with custom AioSession and AioDeferredRefreshableCredentials.

[nicornk]

Change introduced in "Try anon=True if no credentials are supplied or found" broke connection to private S3 compatible storage with custom AioSession and AioDeferredRefreshableCredentials.

[BENR0]

All right sounds good. Do you want to make a PR or do you want me to do it? I probably will have time for this on Friday/the weekend at the earliest.

[jonas-w]

After debugging a little bit I've found the issue, I don't know how this worked ever with our code?

Int this code line s3fs creates a new credential resolver:

s3fs/s3fs/core.py

Line 497 in cc6663f

cred_resolver = create_credential_resolver(

Which is exactly the same as:
https://github.com/aio-libs/aiobotocore/blob/ab08687fdedd5407ab53315716c097543957c684/aiobotocore/session.py#L49

Which gets set in the session here:
https://github.com/boto/botocore/blob/4b728545d43ff70287ae1d0dce42e995790e6b85/botocore/session.py#L186

Which then can be used with get_credentials:
https://github.com/aio-libs/aiobotocore/blob/ab08687fdedd5407ab53315716c097543957c684/aiobotocore/session.py#L76

So basically If s3fs would replace these lines:

s3fs/s3fs/core.py

Lines 497 to 500 in cc6663f

	cred_resolver = create_credential_resolver(
	self.session, region_name=self.session._last_client_region_used
	)
	credentials = cred_resolver.load_credentials()

with

await self.session.get_credentials()

everything would be the same, except that we can insert our own credentials providers.

@BENR0 Is there a specific reason that s3fs creates its own credentials_resolver here? While it is the exact same that gets registered on the aiobotocore session.

[BENR0]

No there is not. My experience with (aio)botocore is very limited and how this is implemented is purely based on what I "puzzled" together. I am very curious why you get the error because the credential_resolver gets created with your session so should basically do the same? But it seems that I am missing something. Apart from that obviously there is a unit test missing for that case.

That said from a first glimpse it seems like your proposed solution could work. Did you have a chance to test if that solves your problem?

[jonas-w]

No there is not. My experience with (aio)botocore is very limited and how this is implemented is purely based on what I "puzzled" together. I am very curious why you get the error because the credential_resolver gets created with your session so should basically do the same? But it seems that I am missing something. Apart from that obviously there is a unit test missing for that case.

I also don't have much experience with (aio)botocore, but this is what I found out after debugging this issue.

Basically, we add the credential provider to the credential resolver instance that is registered in the AioSession. What you do in your code is creating a completely new instance of a credential resolver which is seemingly "connected" (as in, it has the session as a parameter when creating) to the session, but not "registered" as a "component" in the session (as in, it is added to the component list of the session).

That's what's happening in these lines of code:

1. It first creates the credential resolver in the same way you've created it here:
   https://github.com/boto/botocore/blob/5a02e644c0bc86dd9bac6227154cc765dabf85a9/botocore/session.py#L191-L194
2. And then registers it lazily in the session:
   https://github.com/boto/botocore/blob/5a02e644c0bc86dd9bac6227154cc765dabf85a9/botocore/session.py#L186-L189

So creating a new credential provider instance seems to be not needed, as it is already registered as a component in the session.
And we only have access to the credential provider that is registered in the session, without patching the s3fs code directly.

That said from a first glimpse it seems like your proposed solution could work. Did you have a chance to test if that solves your problem?

Yes it solved my problem, I also found an issue in my Credential Provider implementation, but after that with my proposed solution works.

[jonas-w]

All right sounds good. Do you want to make a PR or do you want me to do it? I probably will have time for this on Friday/the weekend at the earliest.

I'll give creating the PR a shot. Thanks!

EDIT: nvm it seems like your PR got reverted yesterday :/ so it's now working again

[nicornk]

Closing since it's reverted