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

Remove and investigate xfail added in #680 #681

Open
Carreau opened this issue Jan 1, 2021 · 3 comments
Open

Remove and investigate xfail added in #680 #681

Carreau opened this issue Jan 1, 2021 · 3 comments

Comments

@Carreau
Copy link
Contributor

Carreau commented Jan 1, 2021

#680 marked a few tests as xfailed when migrating to a newer version of azurite; which apparently does not support empty string as a key.

We should investigate why, and if possible unmark those test from xfail.

@Carreau
Copy link
Contributor Author

Carreau commented Jan 1, 2021

#682 also remove some coverage we might want to re-enable.

@Carreau
Copy link
Contributor Author

Carreau commented Jan 1, 2021

from https://docs.microsoft.com/en-us/rest/api/storageservices/naming-and-referencing-containers--blobs--and-metadata:

A blob name must be at least one character long and cannot be more than 1,024 characters long, for blobs in Azure Storage.

The Azure Storage emulator supports blob names up to 256 characters long. For more information, see Use the Azure storage emulator for development and testing.

So it looks like we need to ensure that in zarr, likely allow to read older blobs that might be empty, but prevent user to write new one that are to avoid losing access to them when Azure stop accepting those requests which right now likely are for backward compat.

@Carreau
Copy link
Contributor Author

Carreau commented Jan 2, 2021

Ok, tracking things down it looks like the only case we query for an empty blob name is when we want the size of the full store; for which the correct path seem to be to list blobs.

Another way would be to use client.get_container_properties(....).properties.content_length which might be less correct if there are other things in the container, so improbable if the prefix used by zarr is empty.

I've made #686 which should fix tests; and we can update to use the above later on.

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

No branches or pull requests

1 participant