Skip to content

Commit

Permalink
Ignore push() encoding if source is a binary file or stream (#991)
Browse files Browse the repository at this point in the history
The push() docs explicitly say that the "encoding" param is ignored if
the source is bytes or a binary stream. This is useful because the
default is encoding='utf-8'; but we should already know the source
stream is binary, so let that override the default.

Also add a test that "encoding" is ignored in these cases.

Fixes #990
  • Loading branch information
benhoyt committed Aug 16, 2023
1 parent 6dd4d36 commit 5c305b8
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 1 deletion.
6 changes: 5 additions & 1 deletion ops/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -2592,7 +2592,11 @@ def push(
elif isinstance(source, bytes):
file_path.write_bytes(source)
else:
with file_path.open('wb' if encoding is None else 'w', encoding=encoding) as f:
# If source is binary, open file in binary mode and ignore encoding param
is_binary = isinstance(source.read(0), bytes)
open_mode = 'wb' if is_binary else 'w'
open_encoding = None if is_binary else encoding
with file_path.open(open_mode, encoding=open_encoding) as f:
shutil.copyfileobj(cast(IOBase, source), cast(IOBase, f))
os.chmod(file_path, permissions)
except FileNotFoundError as e:
Expand Down
16 changes: 16 additions & 0 deletions test/test_testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -3998,6 +3998,22 @@ def _test_push_and_pull_data(self, original_data, encoding, stream_class):
received_data = infile.read()
self.assertEqual(original_data, received_data)

def test_push_bytes_ignore_encoding(self):
# push() encoding param should be ignored if source is bytes
client = self.client
client.push(f"{self.prefix}/test", b'\x00\x01', encoding='utf-8')
with client.pull(f"{self.prefix}/test", encoding=None) as infile:
received_data = infile.read()
self.assertEqual(received_data, b'\x00\x01')

def test_push_bytesio_ignore_encoding(self):
# push() encoding param should be ignored if source is binary stream
client = self.client
client.push(f"{self.prefix}/test", io.BytesIO(b'\x00\x01'), encoding='utf-8')
with client.pull(f"{self.prefix}/test", encoding=None) as infile:
received_data = infile.read()
self.assertEqual(received_data, b'\x00\x01')

def test_push_and_pull_larger_file(self):
# Intent: to ensure things work appropriately with larger files.
# Larger files may be sent/received in multiple chunks; this should help for
Expand Down

0 comments on commit 5c305b8

Please sign in to comment.