-
Notifications
You must be signed in to change notification settings - Fork 2
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
Allow for externally-supplied storage_options
to S3 loader and further down to Reductionist
#182
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi V - looks fine to me, apart from the whole pre-configured options thing. I'm in the office tomorrow if you want to chat about it.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #182 +/- ##
==========================================
+ Coverage 87.11% 88.11% +1.00%
==========================================
Files 8 8
Lines 613 648 +35
==========================================
+ Hits 534 571 +37
+ Misses 79 77 -2 ☔ View full report in Codecov by Sentry. |
…e and keep its name haha
OK we are in very good shape - this thing does a dance and half, pretty much toasts the bread and spreads the butter too, don't mind the S3 tests failing, those are supposed to fail given Reductionist doesn't support |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi V, all after our chat :) Those two modified tests put in as suggestions. Good to go when after that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
mega, many thanks @davidhassell 🍻 |
Description
This is a locally-constructed, and off the back of #180 version of #178 - merely translating @davidhassell 's work here, but with a twist 😁
What we did here:
Active
now ingests two new kwargsstorage_options=None (type: dict)
andactive_storage_url=None (type: str)
by means of which a user can specify S3 credentials straight into Active (or pass them from a higher level egcf-python
). These can be varied, and tested for - e.g. a collection of items like the credentials needed to get into a standard S3 bucket, or an anon=True bucket. See tests/test_compression.py and tests/test_compression_remote_reductionist.py - there we usestorage_options
andactive_storage_url
kwargs to perfectly reproduce connecting to a localhost/Minio data object from a local Reductionist, and also to connect to a valid yet anon=True bucket via a remote Reductionist (this latter stops at Access Denied since Reductionist is not yet able to support anon=True buckets)Active
andnetcdf_to_zarr
Closes #177 and the valiant #178
Before you get started
Checklist