Merge lp:~gz/brz/buffer_to_memoryview into lp:brz

Proposed by Martin Packman on 2017-05-24
Status: Merged
Approved by: Jelmer Vernooij on 2017-05-30
Approved revision: 6636
Merge reported by: The Breezy Bot
Merged at revision: not available
Proposed branch: lp:~gz/brz/buffer_to_memoryview
Merge into: lp:brz
Diff against target: 195 lines (+59/-25)
4 files modified
breezy/osutils.py (+6/-5)
breezy/tests/file_utils.py (+1/-1)
breezy/tests/test_osutils.py (+16/-14)
breezy/transport/sftp.py (+36/-5)
To merge this branch: bzr merge lp:~gz/brz/buffer_to_memoryview
Reviewer Review Type Date Requested Status
Jelmer Vernooij 2017-05-24 Approve on 2017-05-24
Review via email: mp+324573@code.launchpad.net

Commit message

Use memoryview over buffer for compatibility

Description of the change

The 'buffer' 2to3 fixer was especially unhelpful here, creating invalid code. Pretty simple to adapt our uses though.

Branch also makes the classes TestSendAll, TestPumpFile, and TestPumpStringFile pass on Python 3.

To post a comment you must log in.
Jelmer Vernooij (jelmer) :
review: Approve
The Breezy Bot (the-breezy-bot) wrote :

Running landing tests failed
http://10.242.247.184:8080/job/brz-dev/4/

Martin Packman (gz) wrote :

Paramiko is failing me, have filed and proposed fixes upstream:

<https://github.com/paramiko/paramiko/issues/967>
<https://github.com/paramiko/paramiko/issues/968>

Choice here of monkey patching their py3compat module or dropping the optimisation (which is aimed at other transports).

lp:~gz/brz/buffer_to_memoryview updated on 2017-05-26
6635. By Martin Packman on 2017-05-25

Remove old compatibility hack for pre 1.5.5 paramiko

6636. By Martin Packman on 2017-05-26

Dark hackery to get around paramiko Python 3 compatibility mistakes

Martin Packman (gz) wrote :

Removed an old paramiko compatiblity hack and introduced a new one.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'breezy/osutils.py'
2--- breezy/osutils.py 2017-05-24 19:44:00 +0000
3+++ breezy/osutils.py 2017-05-26 01:01:46 +0000
4@@ -746,11 +746,11 @@
5 # drives).
6 if not segment_size:
7 segment_size = 5242880 # 5MB
8- segments = range(len(bytes) / segment_size + 1)
9+ offsets = range(0, len(bytes), segment_size)
10+ view = memoryview(bytes)
11 write = file_handle.write
12- for segment_index in segments:
13- segment = buffer(bytes, segment_index * segment_size, segment_size)
14- write(segment)
15+ for offset in offsets:
16+ write(view[offset:offset+segment_size])
17
18
19 def file_iterator(input_file, readsize=32768):
20@@ -2176,9 +2176,10 @@
21 """
22 sent_total = 0
23 byte_count = len(bytes)
24+ view = memoryview(bytes)
25 while sent_total < byte_count:
26 try:
27- sent = sock.send(buffer(bytes, sent_total, MAX_SOCKET_CHUNK))
28+ sent = sock.send(view[sent_total:sent_total+MAX_SOCKET_CHUNK])
29 except (socket.error, IOError) as e:
30 if e.args[0] in _end_of_stream_errors:
31 raise errors.ConnectionReset(
32
33=== modified file 'breezy/tests/file_utils.py'
34--- breezy/tests/file_utils.py 2017-05-22 00:56:52 +0000
35+++ breezy/tests/file_utils.py 2017-05-26 01:01:46 +0000
36@@ -33,7 +33,7 @@
37 """Reads size characters from the input (or the rest of the string if
38 size is -1)."""
39 data = self.data.read(size)
40- self.max_read_size = max(self.max_read_size, len(data))
41+ self.max_read_size = max(self.max_read_size or 0, len(data))
42 self.read_count += 1
43 return data
44
45
46=== modified file 'breezy/tests/test_osutils.py'
47--- breezy/tests/test_osutils.py 2017-05-22 00:56:52 +0000
48+++ breezy/tests/test_osutils.py 2017-05-26 01:01:46 +0000
49@@ -16,6 +16,8 @@
50
51 """Tests for the osutils wrapper."""
52
53+from __future__ import absolute_import, division
54+
55 import errno
56 import os
57 import re
58@@ -603,8 +605,8 @@
59 super(TestPumpFile, self).setUp()
60 # create a test datablock
61 self.block_size = 512
62- pattern = '0123456789ABCDEF'
63- self.test_data = pattern * (3 * self.block_size / len(pattern))
64+ pattern = b'0123456789ABCDEF'
65+ self.test_data = pattern * (3 * self.block_size // len(pattern))
66 self.test_data_len = len(self.test_data)
67
68 def test_bracket_block_size(self):
69@@ -616,8 +618,8 @@
70 from_file = file_utils.FakeReadFile(self.test_data)
71 to_file = BytesIO()
72
73- # read (max / 2) bytes and verify read size wasn't affected
74- num_bytes_to_read = self.block_size / 2
75+ # read (max // 2) bytes and verify read size wasn't affected
76+ num_bytes_to_read = self.block_size // 2
77 osutils.pumpfile(from_file, to_file, num_bytes_to_read, self.block_size)
78 self.assertEqual(from_file.get_max_read_size(), num_bytes_to_read)
79 self.assertEqual(from_file.get_read_count(), 1)
80@@ -742,23 +744,23 @@
81
82 def test_empty(self):
83 output = BytesIO()
84- osutils.pump_string_file("", output)
85- self.assertEqual("", output.getvalue())
86+ osutils.pump_string_file(b"", output)
87+ self.assertEqual(b"", output.getvalue())
88
89 def test_more_than_segment_size(self):
90 output = BytesIO()
91- osutils.pump_string_file("123456789", output, 2)
92- self.assertEqual("123456789", output.getvalue())
93+ osutils.pump_string_file(b"123456789", output, 2)
94+ self.assertEqual(b"123456789", output.getvalue())
95
96 def test_segment_size(self):
97 output = BytesIO()
98- osutils.pump_string_file("12", output, 2)
99- self.assertEqual("12", output.getvalue())
100+ osutils.pump_string_file(b"12", output, 2)
101+ self.assertEqual(b"12", output.getvalue())
102
103 def test_segment_size_multiple(self):
104 output = BytesIO()
105- osutils.pump_string_file("1234", output, 2)
106- self.assertEqual("1234", output.getvalue())
107+ osutils.pump_string_file(b"1234", output, 2)
108+ self.assertEqual(b"1234", output.getvalue())
109
110
111 class TestRelpath(tests.TestCase):
112@@ -892,7 +894,7 @@
113 for err in errs:
114 sock = DisconnectedSocket(err)
115 self.assertRaises(errors.ConnectionReset,
116- osutils.send_all, sock, 'some more content')
117+ osutils.send_all, sock, b'some more content')
118
119 def test_send_with_no_progress(self):
120 # See https://bugs.launchpad.net/bzr/+bug/1047309
121@@ -909,7 +911,7 @@
122 return 0
123 sock = NoSendingSocket()
124 self.assertRaises(errors.ConnectionReset,
125- osutils.send_all, sock, 'content')
126+ osutils.send_all, sock, b'content')
127 self.assertEqual(1, sock.call_count)
128
129
130
131=== modified file 'breezy/transport/sftp.py'
132--- breezy/transport/sftp.py 2017-05-25 00:04:21 +0000
133+++ breezy/transport/sftp.py 2017-05-26 01:01:46 +0000
134@@ -85,9 +85,41 @@
135 from paramiko.sftp_file import SFTPFile
136
137
138-_paramiko_version = getattr(paramiko, '__version_info__', (0, 0, 0))
139-# don't use prefetch unless paramiko version >= 1.5.5 (there were bugs earlier)
140-_default_do_prefetch = (_paramiko_version >= (1, 5, 5))
141+# GZ 2017-05-25: Some dark hackery to monkeypatch out issues with paramiko's
142+# Python 3 compatibility code. Replace broken b() and asbytes() code.
143+try:
144+ from paramiko.py3compat import b as _bad
145+ from paramiko.common import asbytes as _bad_asbytes
146+except ImportError:
147+ pass
148+else:
149+ def _b_for_broken_paramiko(s, encoding='utf8'):
150+ """Hacked b() that does not raise TypeError."""
151+ # https://github.com/paramiko/paramiko/issues/967
152+ if not isinstance(s, bytes):
153+ encode = getattr(s, 'encode', None)
154+ if encode is not None:
155+ return encode(encoding)
156+ # Would like to pass buffer objects along, but have to realise.
157+ tobytes = getattr(s, 'tobytes', None)
158+ if tobytes is not None:
159+ return tobytes()
160+ return s
161+
162+ def _asbytes_for_broken_paramiko(s):
163+ """Hacked asbytes() that does not raise Exception."""
164+ # https://github.com/paramiko/paramiko/issues/968
165+ if not isinstance(s, bytes):
166+ encode = getattr(s, 'encode', None)
167+ if encode is not None:
168+ return encode('utf8')
169+ asbytes = getattr(s, 'asbytes', None)
170+ if asbytes is not None:
171+ return asbytes()
172+ return s
173+
174+ _bad.func_code = _b_for_broken_paramiko.func_code
175+ _bad_asbytes.func_code = _asbytes_for_broken_paramiko.func_code
176
177
178 class SFTPLock(object):
179@@ -317,7 +349,6 @@
180 class SFTPTransport(ConnectedTransport):
181 """Transport implementation for SFTP access."""
182
183- _do_prefetch = _default_do_prefetch
184 # TODO: jam 20060717 Conceivably these could be configurable, either
185 # by auto-tuning at run-time, or by a configuration (per host??)
186 # but the performance curve is pretty flat, so just going with
187@@ -414,7 +445,7 @@
188 path = self._remote_path(relpath)
189 f = self._get_sftp().file(path, mode='rb')
190 size = f.stat().st_size
191- if self._do_prefetch and (getattr(f, 'prefetch', None) is not None):
192+ if getattr(f, 'prefetch', None) is not None:
193 f.prefetch(size)
194 return f
195 except (IOError, paramiko.SSHException) as e:

Subscribers

People subscribed via source and target branches