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

Add a Boto3Store class #106

Merged
merged 9 commits into from
Feb 23, 2021
Merged

Add a Boto3Store class #106

merged 9 commits into from
Feb 23, 2021

Conversation

zeroSteiner
Copy link
Contributor

@zeroSteiner zeroSteiner commented May 18, 2020

This implements #84 and adds a Boto3Store class using the boto3 library to use S3 as a storage backend.

The code and tests were largely based on the existing BotoStore backend. I'm hoping these tests pass, but I'm getting some errors running them on my system and I can't quite tell if that's expected or not due to the authentication. If the tests need to be updated just let me know and I'd be happy to take care of that.

I left the UrlMixin out because in addition to a bucket, you'd need a boto3.client instance configured for S3. I couldn't find an obvious way to create this from the bucket instance, so the user would need to initialize that every time, or we could initialize it for them but that starts to make assumptions about how they handle authentication. As a third option we could raise RuntimeError if url_for is called and an client wasn't passed to __init__. I opted to leave it out entirely but if you like one of those options more, just let me know.

Update: The UrlMixin support is all set and all of the unit tests should be passing. One thing I wanted to note regarding the URL support is that the url_valid_time parameter is only used if it's set and the object is not an existing object that is publicly accessible. This allows the user to set public=True and then obtain URLs that will be valid indefinitely as opposed to constrained by the 1 week limit that S3 imposes.

Like the original BotoStore class, the bucket argument is expected to be a boto3 Bucket instance. However, if it's a string it will be used as the bucket name and a new Bucket instance will be created as long as it already exists.

Thanks!

This still requires testing and a few todo list items need to be
completed.
@coveralls
Copy link

coveralls commented May 18, 2020

Coverage Status

Coverage decreased (-68.4%) to 23.731% when pulling fa26c79 on zeroSteiner:feat/boto3-store into f87e2da on mbr:master.

@zeroSteiner zeroSteiner marked this pull request as draft August 27, 2020 01:27
@zeroSteiner
Copy link
Contributor Author

Converted this to a draft. It's been a while since I opened it but I'm actively working towards fixing the unit tests and adding the missing URL store support like the existing BotoStore has.

@zeroSteiner zeroSteiner marked this pull request as ready for review August 29, 2020 19:39
@zeroSteiner
Copy link
Contributor Author

Alright, this is all done now and ready for review. It covers all of the same features in a backwards compatible way as the original BotoStore class including support for a public ACL, the REDUCED_REDUNDANCY storage class, metadata and fetching an objects URL. All of the unit tests for this new code should be passing, but it does look like the project over all is failing still.

@fmarczin fmarczin merged commit 2f9d45a into mbr:master Feb 23, 2021
@zeroSteiner
Copy link
Contributor Author

Thanks for the merge!

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.

3 participants