chore: add test for ZarrParser on bucket without list permissions#919
Conversation
|
Yeah I was thinking about this. For now I think we want some fallback approach (i.e. the old approach), that is opt-in via a kwarg like The main is question is whether we can auto-detect whether or not a store is listable. Zarr stores have a property for this, but we don't have Zarr stores, we have obstore stores, and I don't see a |
where is there a .listable check on zarr stores? Probably a try; except or a parser configuration option is the way to go so that you don't waste a request for the listable path just checking if it's listable. |
https://zarr.readthedocs.io/en/stable/api/zarr/abc/store/#zarr.abc.store.Store.supports_listing I'm fine with a parser kwarg, I just want it to default to not use listing, despite that choice being a breaking change, because for listable stores using list is such a massive performance improvement. |
ah, thanks. that is about the store implementation rather than the permissions of the backend, so it wouldn't help here even if we made a Zarr store from the obstore Store.
do you mean default to use listing? I'd be fine with that. I think this should be well documented - the changelog entry from #892 should be also listed under breaking changes and should go under the unreleased section rather than 2.4.0. |
| reason="ZarrParser does not yet support buckets without list permissions" | ||
| ) | ||
| def test_zarr_parser_nolist_bucket(minio_nolist_bucket): | ||
| """Test that ZarrParser works with a bucket that does not allow list operations.""" |
There was a problem hiding this comment.
I'm fine to merge this as-is, but I wonder if there is a simpler test that doesn't require minio.
The simplest example of a zarr url that we can't list is surely a http store? I was imagining there would be a class like that in zarr-python, but apparently there isn't.
Obstore does have a HTTP store, but I'm confused why it says it supports list?
https://developmentseed.org/obstore/latest/api/store/http/#obstore.store.HTTPStore.head_async
There was a problem hiding this comment.
obstore's HTTPStore is designed around webdav (xref developmentseed/obspec-utils#63).
It's a trade-off between code complexity with minio and relying on the network. For this version, you only need docker locally. I prefer local-first testing but am not planning to implement the fix so you could close this and build what works for you.
There was a problem hiding this comment.
I see. Sounds harder than I had anticipated, so minio seems fine.
|
Thanks for the fix @maxrjones |
|
This doesn't actually fix the problem (which I just raised #921 to track), it only adds the test that we need to make pass. |
3 participants
What I did
@TomNicholas mentioned some recent Zarr parser changes at a call today (#892), and I was curious about the implications for buckets without list permissions (e.g., HTTPS stores). This PR just adds a test that xfails.
As an aside, this is where #859 would come in handy because parsers could define their own required methods.
Acceptance criteria:
docs/releases.md*.mdfile underdocs/api