Merge lp:~simpoir/landscape-client/py3fix-network-usage into lp:~landscape/landscape-client/trunk

Proposed by Simon Poirier on 2017-04-19
Status: Merged
Approved by: Simon Poirier on 2017-04-20
Approved revision: 1018
Merged at revision: 1020
Proposed branch: lp:~simpoir/landscape-client/py3fix-network-usage
Merge into: lp:~landscape/landscape-client/trunk
Diff against target: 197 lines (+51/-22)
6 files modified
landscape/broker/store.py (+2/-1)
landscape/lib/amp.py (+2/-1)
landscape/lib/bpickle.py (+22/-16)
landscape/lib/tests/test_amp.py (+15/-0)
landscape/lib/tests/test_bpickle.py (+8/-2)
landscape/monitor/tests/test_networkactivity.py (+2/-2)
To merge this branch: bzr merge lp:~simpoir/landscape-client/py3fix-network-usage
Reviewer Review Type Date Requested Status
Landscape Builder test results Approve on 2017-04-19
Eric Snow (community) 2017-04-19 Approve on 2017-04-19
Review via email: mp+322794@code.launchpad.net

Commit message

This branch fixes reporting of network usage under python3. It adds a parameter to bpicle to allow loading without casting bytes keys to string in dict in the cases where data need to be passed as-is (broker and messaging serialization).

Description of the change

This branch fixes reporting of network usage under python3. It adds a parameter to bpicle to allow loading without casting bytes keys to string in dict in the cases where data need to be passed as-is (broker and messaging serialization).

Testing instructions:

make check2 check3
./scripts/landscape-client -c root-config

Accept the client and check in Monitoring for network graphs a few minutes later.

To post a comment you must log in.
review: Abstain (executing tests)

Command: TRIAL_ARGS=-j4 make ci-check
Result: Fail
Revno: 1018
Branch: lp:~simpoir/landscape-client/py3fix-network-usage
Jenkins: https://ci.lscape.net/job/latch-test-xenial/3877/

review: Needs Fixing (test results)
Eric Snow (ericsnowcurrently) wrote :

LGTM

Interestingly, we had something similar before but then someone-I-won't-name decided to simplify... :)

review: Approve
review: Abstain (executing tests)

Command: TRIAL_ARGS=-j4 make ci-check
Result: Success
Revno: 1018
Branch: lp:~simpoir/landscape-client/py3fix-network-usage
Jenkins: https://ci.lscape.net/job/latch-test-xenial/3878/

review: Approve (test results)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'landscape/broker/store.py'
--- landscape/broker/store.py 2017-04-06 18:27:36 +0000
+++ landscape/broker/store.py 2017-04-19 20:54:09 +0000
@@ -262,7 +262,8 @@
262 break262 break
263 data = read_binary_file(self._message_dir(filename))263 data = read_binary_file(self._message_dir(filename))
264 try:264 try:
265 message = bpickle.loads(data)265 # don't reinterpret messages that are meant to be sent out
266 message = bpickle.loads(data, as_is=True)
266 except ValueError as e:267 except ValueError as e:
267 logging.exception(e)268 logging.exception(e)
268 self._add_flags(filename, BROKEN)269 self._add_flags(filename, BROKEN)
269270
=== modified file 'landscape/lib/amp.py'
--- landscape/lib/amp.py 2017-03-17 09:26:45 +0000
+++ landscape/lib/amp.py 2017-04-19 20:54:09 +0000
@@ -165,7 +165,8 @@
165 chunks.append(arguments)165 chunks.append(arguments)
166 arguments = b"".join(chunks)166 arguments = b"".join(chunks)
167167
168 args, kwargs = bpickle.loads(arguments)168 # Pass the the arguments as-is without reinterpreting strings.
169 args, kwargs = bpickle.loads(arguments, as_is=True)
169170
170 # We encoded the method name in `send_method_call` and have to decode171 # We encoded the method name in `send_method_call` and have to decode
171 # it here again.172 # it here again.
172173
=== modified file 'landscape/lib/bpickle.py'
--- landscape/lib/bpickle.py 2017-04-11 14:13:28 +0000
+++ landscape/lib/bpickle.py 2017-04-19 20:54:09 +0000
@@ -45,13 +45,19 @@
45 raise ValueError("Unsupported type: %s" % e)45 raise ValueError("Unsupported type: %s" % e)
4646
4747
48def loads(byte_string, _lt=loads_table):48def loads(byte_string, _lt=loads_table, as_is=False):
49 """Load a serialized byte_string.
50
51 @param byte_string: the serialized data
52 @param _lt: the conversion map
53 @param as_is: don't reinterpret dict keys as str
54 """
49 if not byte_string:55 if not byte_string:
50 raise ValueError("Can't load empty string")56 raise ValueError("Can't load empty string")
51 try:57 try:
52 # To avoid python3 turning byte_string[0] into an int,58 # To avoid python3 turning byte_string[0] into an int,
53 # we slice the bytestring instead.59 # we slice the bytestring instead.
54 return _lt[byte_string[0:1]](byte_string, 0)[0]60 return _lt[byte_string[0:1]](byte_string, 0, as_is=as_is)[0]
55 except KeyError as e:61 except KeyError as e:
56 raise ValueError("Unknown type character: %s" % e)62 raise ValueError("Unknown type character: %s" % e)
57 except IndexError:63 except IndexError:
@@ -107,59 +113,59 @@
107 return b"n"113 return b"n"
108114
109115
110def loads_bool(bytestring, pos):116def loads_bool(bytestring, pos, as_is=False):
111 return bool(int(bytestring[pos+1:pos+2])), pos+2117 return bool(int(bytestring[pos+1:pos+2])), pos+2
112118
113119
114def loads_int(bytestring, pos):120def loads_int(bytestring, pos, as_is=False):
115 endpos = bytestring.index(b";", pos)121 endpos = bytestring.index(b";", pos)
116 return int(bytestring[pos+1:endpos]), endpos+1122 return int(bytestring[pos+1:endpos]), endpos+1
117123
118124
119def loads_float(bytestring, pos):125def loads_float(bytestring, pos, as_is=False):
120 endpos = bytestring.index(b";", pos)126 endpos = bytestring.index(b";", pos)
121 return float(bytestring[pos+1:endpos]), endpos+1127 return float(bytestring[pos+1:endpos]), endpos+1
122128
123129
124def loads_bytes(bytestring, pos):130def loads_bytes(bytestring, pos, as_is=False):
125 startpos = bytestring.index(b":", pos)+1131 startpos = bytestring.index(b":", pos)+1
126 endpos = startpos+int(bytestring[pos+1:startpos-1])132 endpos = startpos+int(bytestring[pos+1:startpos-1])
127 return bytestring[startpos:endpos], endpos133 return bytestring[startpos:endpos], endpos
128134
129135
130def loads_unicode(bytestring, pos):136def loads_unicode(bytestring, pos, as_is=False):
131 startpos = bytestring.index(b":", pos)+1137 startpos = bytestring.index(b":", pos)+1
132 endpos = startpos+int(bytestring[pos+1:startpos-1])138 endpos = startpos+int(bytestring[pos+1:startpos-1])
133 return bytestring[startpos:endpos].decode("utf-8"), endpos139 return bytestring[startpos:endpos].decode("utf-8"), endpos
134140
135141
136def loads_list(bytestring, pos, _lt=loads_table):142def loads_list(bytestring, pos, _lt=loads_table, as_is=False):
137 pos += 1143 pos += 1
138 res = []144 res = []
139 append = res.append145 append = res.append
140 while bytestring[pos:pos+1] != b";":146 while bytestring[pos:pos+1] != b";":
141 obj, pos = _lt[bytestring[pos:pos+1]](bytestring, pos)147 obj, pos = _lt[bytestring[pos:pos+1]](bytestring, pos, as_is=as_is)
142 append(obj)148 append(obj)
143 return res, pos+1149 return res, pos+1
144150
145151
146def loads_tuple(bytestring, pos, _lt=loads_table):152def loads_tuple(bytestring, pos, _lt=loads_table, as_is=False):
147 pos += 1153 pos += 1
148 res = []154 res = []
149 append = res.append155 append = res.append
150 while bytestring[pos:pos+1] != b";":156 while bytestring[pos:pos+1] != b";":
151 obj, pos = _lt[bytestring[pos:pos+1]](bytestring, pos)157 obj, pos = _lt[bytestring[pos:pos+1]](bytestring, pos, as_is=as_is)
152 append(obj)158 append(obj)
153 return tuple(res), pos+1159 return tuple(res), pos+1
154160
155161
156def loads_dict(bytestring, pos, _lt=loads_table):162def loads_dict(bytestring, pos, _lt=loads_table, as_is=False):
157 pos += 1163 pos += 1
158 res = {}164 res = {}
159 while bytestring[pos:pos+1] != b";":165 while bytestring[pos:pos+1] != b";":
160 key, pos = _lt[bytestring[pos:pos+1]](bytestring, pos)166 key, pos = _lt[bytestring[pos:pos+1]](bytestring, pos, as_is=as_is)
161 val, pos = _lt[bytestring[pos:pos+1]](bytestring, pos)167 val, pos = _lt[bytestring[pos:pos+1]](bytestring, pos, as_is=as_is)
162 if _PY3 and isinstance(key, bytes):168 if _PY3 and not as_is and isinstance(key, bytes):
163 # Although the wire format of dictionary keys is ASCII bytes, the169 # Although the wire format of dictionary keys is ASCII bytes, the
164 # code actually expects them to be strings, so we convert them170 # code actually expects them to be strings, so we convert them
165 # here.171 # here.
@@ -168,7 +174,7 @@
168 return res, pos+1174 return res, pos+1
169175
170176
171def loads_none(str, pos):177def loads_none(str, pos, as_is=False):
172 return None, pos+1178 return None, pos+1
173179
174180
175181
=== modified file 'landscape/lib/tests/test_amp.py'
--- landscape/lib/tests/test_amp.py 2017-03-09 13:41:17 +0000
+++ landscape/lib/tests/test_amp.py 2017-04-19 20:54:09 +0000
@@ -213,6 +213,21 @@
213 self.connection.flush()213 self.connection.flush()
214 self.assertEqual("barfoobarfoobarfoo", self.successResultOf(deferred))214 self.assertEqual("barfoobarfoobarfoo", self.successResultOf(deferred))
215215
216 def test_with_bytes_dictionary_arguments(self):
217 """
218 Method arguments passed to a MethodCall can be a dict of bytes.
219 """
220 arg = {b"byte_key": 1}
221 self.object.method = lambda d: ",".join([
222 type(x).__name__ for x in d.keys()])
223 deferred = self.sender.send_method_call(
224 method="method",
225 args=[arg],
226 kwargs={})
227 self.connection.flush()
228 # str under python2, bytes under python3
229 self.assertEqual(type(b"").__name__, self.successResultOf(deferred))
230
216 def test_with_non_serializable_return_value(self):231 def test_with_non_serializable_return_value(self):
217 """232 """
218 If the target object method returns an object that can't be serialized,233 If the target object method returns an object that can't be serialized,
219234
=== modified file 'landscape/lib/tests/test_bpickle.py'
--- landscape/lib/tests/test_bpickle.py 2017-04-10 21:28:13 +0000
+++ landscape/lib/tests/test_bpickle.py 2017-04-19 20:54:09 +0000
@@ -49,8 +49,14 @@
49 {True: False})49 {True: False})
5050
51 def test_dict_bytes_keys(self):51 def test_dict_bytes_keys(self):
52 data = bpickle.dumps({b"hello": True})52 """Check loading dict bytes keys without reinterpreting."""
53 self.assertEqual(bpickle.loads(data), {"hello": True})53 # Happens in amp and broker. Since those messages are meant to be
54 # forwarded to the server without changing schema, keys shouldn't be
55 # decoded in this case.
56 initial_data = {b"hello": True}
57 data = bpickle.dumps(initial_data)
58 result = bpickle.loads(data, as_is=True)
59 self.assertEqual(initial_data, result)
5460
55 def test_long(self):61 def test_long(self):
56 long = 9999999999999999999999999999962 long = 99999999999999999999999999999
5763
=== modified file 'landscape/monitor/tests/test_networkactivity.py'
--- landscape/monitor/tests/test_networkactivity.py 2017-04-05 13:08:02 +0000
+++ landscape/monitor/tests/test_networkactivity.py 2017-04-19 20:54:09 +0000
@@ -184,8 +184,8 @@
184 self.assertMessages(self.mstore.get_pending_messages(),184 self.assertMessages(self.mstore.get_pending_messages(),
185 [{"type": "network-activity",185 [{"type": "network-activity",
186 "activities": {186 "activities": {
187 "lo": [(step_size, 0, 1000)],187 b"lo": [(step_size, 0, 1000)],
188 "eth0": [(step_size, 0, 1000)]}}])188 b"eth0": [(step_size, 0, 1000)]}}])
189189
190 def test_config(self):190 def test_config(self):
191 """The network activity plugin is enabled by default."""191 """The network activity plugin is enabled by default."""

Subscribers

People subscribed via source and target branches

to all changes: