Skip to content

Commit

Permalink
fixed pickle tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@shikharsg
shikharsg committed Aug 15, 2018
1 parent f66dadd commit b8f60fe
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 13 deletions.
16 changes: 9 additions & 7 deletions zarr/storage.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1911,14 +1911,16 @@ class ABSStore(MutableMapping):
`Python Client Library <https://github.com/Azure/azure-storage-python/tree/master/azure-storage-blob>`_ version >= 1.3.0.
"""

def __init__(self, container_name, prefix, account_name, account_key):
def __init__(self, container, prefix, account_name=None, account_key=None, blob_service_kwargs=None):
self.container_name = container
self.prefix = normalize_storage_path(prefix)
self.account_name = account_name
self.account_key = account_key
self.container_name = container_name
self.prefix = normalize_storage_path(prefix)

def initialize_container(self):
self.client = BlockBlobService(self.account_name, self.account_key)
if blob_service_kwargs is not None:
self.blob_service_kwargs = blob_service_kwargs
else:
self.blob_service_kwargs = dict()
self.client = BlockBlobService(self.account_name, self.account_key, **self.blob_service_kwargs)

# needed for pickling
def __getstate__(self):
Expand All @@ -1927,7 +1929,7 @@ def __getstate__(self):

def __setstate__(self, state):
self.__dict__.update(state)
self.initialize_container()
self.client = BlockBlobService(self.account_name, self.account_key, **self.blob_service_kwargs)

def __enter__(self):
return self
Expand Down
4 changes: 2 additions & 2 deletions zarr/tests/test_core.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1219,8 +1219,8 @@ def absstore():
blob_client = BlockBlobService(is_emulated=True)
if not blob_client.exists('test'):
blob_client.create_container('test')
store = ABSStore(container_name='test', prefix='zarrtesting/', account_name='foo', account_key='bar')
store.client = blob_client
store = ABSStore(container='test', prefix='zarrtesting/', account_name='foo', account_key='bar',
blob_service_kwargs={'is_emulated':True})
store.rmdir()
return store

Expand Down
4 changes: 2 additions & 2 deletions zarr/tests/test_hierarchy.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -864,8 +864,8 @@ def create_store():
blob_client = BlockBlobService(is_emulated=True)
if not blob_client.exists('test'):
blob_client.create_container('test')
store = ABSStore(container_name='test', prefix='zarrtesting/', account_name='foo', account_key='bar')
store.client = blob_client
store = ABSStore(container='test', prefix='zarrtesting/', account_name='foo', account_key='bar',
blob_service_kwargs={'is_emulated': True})
store.rmdir()
return store, None

Expand Down
4 changes: 2 additions & 2 deletions zarr/tests/test_storage.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1244,7 +1244,7 @@ def create_store(self):
blob_client = BlockBlobService(is_emulated=True)
if not blob_client.exists('test'):
blob_client.create_container('test')
store = ABSStore(container_name='test', prefix='zarrtesting/', account_name='foo', account_key='bar')
store.client = blob_client
store = ABSStore(container='test', prefix='zarrtesting/', account_name='foo', account_key='bar',
blob_service_kwargs={'is_emulated':True})
store.rmdir()
return store

3 comments on commit b8f60fe

@shikharsg
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tjcrone. I have fixed the testing part by allowing for a blob service "kwargs" option to ABSStore which gets passed to BlockBlobService. This way the user never has to deal with the client and it is usable in the way you originally wrote it.

For testing one can simply set the blob_service_kwargs dictionary to blob_service_kwargs={'is_emulated':True}. This is how I've used ABSStore in testing above, and the user can also test it within their own app using the same method.

I have also fixed listdir and getsize in future commits with all tests passing. What's left to implement is rename, although strictly speaking that's optional. Meanwhile, if you give the permission, I can submit a pull request onto the zarr-developers ABSStore branch for everyone to better see the diff.

@tjcrone
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a kwargs option is a great solution. @rabernat has a kwargs argument in his implementation of a GCSStore, which has not been merged yet. I think he has some testing components there as well. It would definitely be worthwhile looking at how he did things and make sure that we are doing similar tests if they look like they will work for us.

@tjcrone
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shikharsg, let's go ahead with a PR to the abs_store branch so that others can check it out and comment. Much appreciate your work on this!

Please sign in to comment.