Skip to content

Comments

Make import of s3fs conditional in aws_credentials.py#30

Open
huonw wants to merge 2 commits intodazza-codes:mainfrom
huonw:conditional-s3fs
Open

Make import of s3fs conditional in aws_credentials.py#30
huonw wants to merge 2 commits intodazza-codes:mainfrom
huonw:conditional-s3fs

Conversation

@huonw
Copy link

@huonw huonw commented Nov 29, 2023

This PR adjusts the aws_credentials.py file to move the from s3fs ... import to be conditional: if the s3fs library doesn't exist, the code can still work fine, because the import is just to get an object that needs its cache cleared, and if there's no object, there's no need to clear anything. This PR also then restores the s3fs library to be optional (revising #26 and partially reverting #27).

Before this PR, the from s3fs import ... at the top level meant that the s3fs library had to be installed in order to import aws_credentials.py, which I think isn't the intention from the aws_s3fs.py code, which carefully guards the imports.

Having the dependency be optional is nice for us: fewer dependencies is better, and we're having trouble with s3fs and related libraries putting overly restrictive version ranges on dependencies like aiobotocore. So, if we don't need to depend on s3fs (because we don't use it), that'd be great. :)

Thanks for pytest-aiomoto, it's been handy for us. Let me know what you think!

@huonw
Copy link
Author

huonw commented Nov 29, 2023

Ah, without the s3fs import, I think this no longer has a (required) dependency on aiobotocore, but that one is definitely required. Before I spend effort getting that arranged, I'll wait to agree on the direction.

One option would be having a direct dependency on aiobotocore = "...", same as moto?

@huonw
Copy link
Author

huonw commented Dec 6, 2023

Heya @dazza-codes, thanks for this library. Do you have thoughts on this PR? (Just a gentle ping in case you missed it 😄 )

@dazza-codes
Copy link
Owner

@huonw - I haven't been watching this repo for a while, sorry for a very tardy reply to this PR.

IMHO, the tight pinning among the aioboto* libs comes down to the way aiobotocore hacks private content in botocore, so it tightly pins to the botocore release that is hacked in case the private details change. Until that changes, if it ever changes, the entire ecosystem of pandas->s3fs->aiobotocore is in a bind; and consequently this test plugin too.

Using package groups in your project, you should be able to limit the dependency on this test plugin to the test group, and therefore also limit s3fs to the test group. But maybe even that introduces constraints on your entire dep-tree.

IMHO, I don't think this test plugin can make s3fs an optional dependency. Although I started out that way, I've arrived at the belief that using s3fs[*] with all extras is the most accurate/efficient way to depend on aiobotocore as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants