8000 Merge pull request #526 from shikharsg/abs_store_fix · davidbrochart/zarr-python@f228883 · GitHub 7FFF
[go: up one dir, main page]
Skip to content

Commit f228883

Browse files
shikharsgshikharsg
authored
Merge pull request zarr-developers#526 from shikharsg/abs_store_fix
Fixed '/' prepend bug in ABSStore
2 parents 121e9ec + c24407c commit f228883

File tree

5 files changed

+75
-30
lines changed

5 files changed

+75
-30
lines changed

‎docs/release.rst

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,13 @@ Upcoming Release
5555
* Add documentation build to CI.
5656
By :user:`James Bourbeau <jrbourbeau>`; :issue:`516`.
5757

58+
8000 Bug fixes
59+
~~~~~~~~~
60+
61+
* Fix '/' prepend bug in ``ABSStore``.
62+
By :user:`Shikhar Goenka <shikharsg>`; :issue:`525`.
63+
64+
5865
.. _release_2.3.2:
5966

6067
2.3.2

‎zarr/storage.py

Lines changed: 37 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1964,7 +1964,7 @@ class ABSStore(MutableMapping):
19641964
In order to use this store, you must install the Microsoft Azure Storage SDK for Python.
19651965
"""
19661966

1967-
def __init__(self, container, prefix, account_name=None, account_key=None,
1967+
def __init__(self, container, prefix='', account_name=None, account_key=None,
19681968
blob_service_kwargs=None):
19691969
from azure.storage.blob import BlockBlobService
19701970
self.container = container
@@ -1990,21 +1990,25 @@ def __setstate__(self, state):
19901990
self.client = BlockBlobService(self.account_name, self.account_key,
19911991
**self.blob_service_kwargs)
19921992

1993-
@staticmethod
1994-
def _append_path_to_prefix(path, prefix):
1995-
return '/'.join([normalize_storage_path(prefix),
1996-
normalize_storage_path(path)])
1993+
def _append_path_to_prefix(self, path):
1994+
if self.prefix == '':
1995+
return normalize_storage_path(path)
1996+
else:
1997+
return '/'.join([self.prefix, normalize_storage_path(path)])
19971998

19981999
@staticmethod
19992000
def _strip_prefix_from_path(path, prefix):
20002001
# normalized things will not have any leading or trailing slashes
20012002
path_norm = normalize_storage_path(path)
20022003
prefix_norm = normalize_storage_path(prefix)
2003-
return path_norm[(len(prefix_norm)+1):]
2004+
if prefix:
2005+
return path_norm[(len(prefix_norm)+1):]
2006+
else:
2007+
return path_norm
20042008

20052009
def __getitem__(self, key):
20062010
from azure.common import AzureMissingResourceHttpError
2007-
blob_name = '/'.join([self.prefix, key])
2011+
blob_name = self._append_path_to_prefix(key)
20082012
try:
20092013
blob = self.client.get_blob_to_bytes(self.container, blob_name)
20102014
return blob.content
@@ -2013,13 +2017,13 @@ def __getitem__(self, key):
20132017

20142018
def __setitem__(self, key, value):
20152019
value = ensure_bytes(value)
2016-
blob_name = '/'.join([self.prefix, key])
2020+
blob_name = self._append_path_to_prefix(key)
20172021
self.client.create_blob_from_bytes(self.container, blob_name, value)
20182022

20192023
def __delitem__(self, key):
20202024
from azure.common import AzureMissingResourceHttpError
20212025
try:
2022-
self.client.delete_blob(self.container, '/'.join([self.prefix, key]))
2026+
self.client.delete_blob(self.container, self._append_path_to_prefix(key))
20232027
except AzureMissingResourceHttpError:
20242028
raise KeyError('Blob %s not found' % key)
20252029

20342038
return list(self.__iter__())
20352039

20362040
def __iter__(self):
2037-
for blob in self.client.list_blobs(self.container, self.prefix + '/'):
2041+
if self.prefix:
2042+
list_blobs_prefix = self.prefix + '/'
2043+
else:
2044+
list_blobs_prefix = None
2045+
for blob in self.client.list_blobs(self.container, list_blobs_prefix):
20382046
yield self._strip_prefix_from_path(blob.name, self.prefix)
20392047

20402048
def __len__(self):
20412049
return len(self.keys())
20422050

20432051
def __contains__(self, key):
2044-
blob_name = '/'.join([self.prefix, key])
2052+
blob_name = self._append_path_to_prefix(key)
20452053
if self.client.exists(self.container, blob_name):
20462054
return True
20472055
else:
20482056
return False
20492057

20502058
def listdir(self, path=None):
2051-
store_path = normalize_storage_path(path)
2052-
# prefix is normalized to not have a trailing slash
2053-
dir_path = self.prefix
2054-
if store_path:
2055-
dir_path = dir_path + '/' + store_path
2056-
dir_path += '/'
2059+
from azure.storage.blob import Blob
2060+
dir_path = normalize_storage_path(self._append_path_to_prefix(path))
2061+
if dir_path:
2062+
dir_path += '/'
20572063
items = list()
20582064
for blob in self.client.list_blobs(self.container, prefix=dir_path, delimiter='/'):
2059-
if '/' in blob.name[len(dir_path):]:
2065+
if type(blob) == Blob:
2066+
items.append(self._strip_prefix_from_path(blob.name, dir_path))
2067+
else:
20602068
items.append(self._strip_prefix_from_path(
20612069
blob.name[:blob.name.find('/', len(dir_path))], dir_path))
2062-
else:
2063-
items.append(self._strip_prefix_from_path(blob.name, dir_path))
20642070
return items
20652071

20662072
def rmdir(self, path=None):
2067-
dir_path = normalize_storage_path(self._append_path_to_prefix(path, self.prefix)) + '/'
2073+
dir_path = normalize_storage_path(self._append_path_to_prefix(path))
2074+
if dir_path:
2075+
dir_path += '/'
20682076
for blob in self.client.list_blobs(self.container, prefix=dir_path):
20692077
self.client.delete_blob(self.container, blob.name)
20702078

20712079
def getsize(self, path=None):
2080+
from azure.storage.blob import Blob
20722081
store_path = normalize_storage_path(path)
20732082
fs_path = self.prefix
20742083
if store_path:
2075-
fs_path = self._append_path_to_prefix(store_path, self.prefix)
2084+
fs_path = self._append_path_to_prefix(store_path)
20762085
if self.client.exists(self.container, fs_path):
20772086
return self.client.get_blob_properties(self.container,
20782087
fs_path).properties.content_length
20792088
else:
20802089
size = 0
2081-
for blob in self.client.list_blobs(self.container, prefix=fs_path + '/',
2090+
if fs_path == '':
2091+
fs_path = None
2092+
else:
2093+
fs_path += '/'
2094+
for blob in self.client.list_blobs(self.container, prefix=fs_path,
20822095
delimiter='/'):
2083< F438 span class="diff-text-marker">-
if '/' not in blob.name[len(fs_path + '/'):]:
2096+
if type(blob) == Blob:
20842097
size += blob.properties.content_length
20852098
return size
20862099

‎zarr/tests/test_core.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1404,8 +1404,8 @@ def absstore():
14041404
blob_client = asb.BlockBlobService(is_emulated=True)
14051405
blob_client.delete_container('test')
14061406
blob_client.create_container('test')
1407-
store = ABSStore(container='test', prefix='zarrtesting/', account_name='foo',
1408-
account_key='bar', blob_service_kwargs={'is_emulated': True})
1407+
store = ABSStore(container='test', account_name='foo', account_key='bar',
1408+
blob_service_kwargs={'is_emulated': True})
14091409
store.rmdir()
14101410
return store
14111411

‎zarr/tests/test_hierarchy.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -894,8 +894,8 @@ def create_store():
894894
blob_client = asb.BlockBlobService(is_emulated=True)
895895
blob_client.delete_container('test')
896896
blob_client.create_container('test')
897-
store = ABSStore(container='test', prefix='zarrtesting/', account_name='foo',
898-
account_key='bar', blob_service_kwargs={'is_emulated': True})
897+
store = ABSStore(container='test', account_name='foo', account_key='bar',
898+
blob_service_kwargs={'is_emulated': True})
899899
store.rmdir()
900900
return store, None
901901

‎zarr/tests/test_storage.py

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1473,16 +1473,41 @@ def test_format_compatibility():
14731473
@skip_test_env_var("ZARR_TEST_ABS")
14741474
class TestABSStore(StoreTests, unittest.TestCase):
14751475

1476-
def create_store(self):
1476+
def create_store(self, prefix=None):
14771477
asb = pytest.importorskip("azure.storage.blob")
14781478
blob_client = asb.BlockBlobService(is_emulated=True)
14791479
blob_client.delete_container('test')
14801480
blob_client.create_container('test')
1481-
store = ABSStore(container='test', prefix='zarrtesting/', account_name='foo',
1481+
store = ABSStore(container='test', prefix=prefix, account_name='foo',
14821482
account_key='bar', blob_service_kwargs={'is_emulated': True})
14831483
store.rmdir()
14841484
return store
14851485

1486+
def test_iterators_with_prefix(self):
1487+
for prefix in ['test_prefix', '/test_prefix', 'test_prefix/', 'test/prefix', '', None]:
1488+
store = self.create_store(prefix=prefix)
1489+
1490+
# test iterator methods on empty store
1491+
assert 0 == len(store)
1492+
assert set() == set(store)
1493+
assert set() == set(store.keys())
1494+
assert set() == set(store.values())
1495+
assert set() == set(store.items())
1496+
1497+
# setup some values
1498+
store['a'] = b'aaa'
1499+
store['b'] = b'bbb'
1500+
store['c/d'] = b'ddd'
1501+
store['c/e/f'] = b'fff'
1502+
1503+
# test iterators on store with data
1504+
assert 4 == len(store)
1505+
assert {'a', 'b', 'c/d', 'c/e/f'} == set(store)
1506+
assert {'a', 'b', 'c/d', 'c/e/f'} == set(store.keys())
1507+
assert {b'aaa', b'bbb', b'ddd', b'fff'} == set(store.values())
1508+
assert ({('a', b'aaa'), ('b', b'bbb'), ('c/d', b'ddd'), ('c/e/f', b'fff')} ==
1509+
set(store.items()))
1510+
14861511

14871512
class TestConsolidatedMetadataStore(unittest.TestCase):
14881513

0 commit comments

Comments
 (0)
0