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.

Sign up for GitHub

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

Fix off by one, raise ValueError on closed file and set readable = True #69

Merged
merged 3 commits into from
Nov 28, 2017
Merged

Fix off by one, raise ValueError on closed file and set readable = True #69

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7f01e01
raise Value Error on closed file
hoffmann Nov 23, 2017
69d320c
fix off by one error
hoffmann Nov 23, 2017
c31aa84
update changelog
hoffmann Nov 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/changes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
Changelog
*********
0.11.5
======
* Fix one off in open() method interfaces for azure backend

0.11.4
======
Expand Down
11 changes: 10 additions & 1 deletion simplekv/net/azurestore.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,16 +142,20 @@ def __init__(self, block_blob_service, container_name, key):

def tell(self):
"""Returns he current offset as int. Always >= 0."""
if self.closed:
raise ValueError("I/O operation on closed file")
return self.pos

def read(self, size=-1):
"""Returns 'size' amount of bytes or less if there is no more data.
If no size is given all data is returned. size can be >= 0."""
if self.closed:
raise ValueError("I/O operation on closed file")
with map_azure_exceptions(key=self.key):
if size < 0:
size = self.size - self.pos

end = min(self.pos + size - 1, self.size)
end = min(self.pos + size - 1, self.size - 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we have a test which fails on current master and succeeds with the change?

Copy link
Collaborator Author

@hoffmann hoffmann Nov 23, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure. The azure API does not care if you request more than the actual size. It just gives you the all the bytes it can. So in my opinion the current implementation should have also worked fine. So the new implementation is only more correct, but behaves same as the previous one.

block_blob_service = BlockBlobService(account_name=ACCOUNT_NAME, account_key=ACCOUNT_KEY)
b = block_blob_service.get_blob_properties("parquet", "test")
size = b.properties.content_length
b = block_blob_service.get_blob_to_bytes("parquet", "test", start_range=size-1, end_range=size)
b.content

if self.pos > end:
return b''
b = self.block_blob_service.get_blob_to_bytes(
Expand All @@ -173,6 +177,8 @@ def seek(self, offset, whence=0):
Any seek operation which moves the position after the stream
should succeed. tell() should report that position and read()
should return an empty bytes object."""
if self.closed:
raise ValueError("I/O operation on closed file")
if whence == 0:
if offset < 0:
raise IOError('seek would move position outside the file')
Expand All @@ -188,3 +194,6 @@ def seek(self, offset, whence=0):

def seekable(self):
return True

def readable(self):
return True
9 changes: 9 additions & 0 deletions tests/test_azure_store.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ def test_open_seek_and_tell(self, store, key, long_value):
store.put(key, long_value)
ok = store.open(key)
assert ok.seekable()
assert ok.readable()
ok.seek(10)
assert ok.tell() == 10
ok.seek(-6, 1)
Expand All @@ -75,6 +76,14 @@ def test_open_seek_and_tell(self, store, key, long_value):
assert ok.tell() == length_lv + 10
assert len(ok.read()) == 0

ok.close()
with pytest.raises(ValueError):
ok.tell()
with pytest.raises(ValueError):
ok.read(1)
with pytest.raises(ValueError):
ok.seek(10)


class TestExtendedKeysAzureStorage(TestAzureStorage, ExtendedKeyspaceTests):
@pytest.fixture
Expand Down