Merge lp:~gocept/landscape-client/py3-amp into lp:~landscape/landscape-client/trunk

[Steffen Allner]

Update code in the landscape.lib.amp module to support Python2/3 which provides the basic functionality to execute methods on remote Python objects via amp. As such it provides an integral part of the system.

As all information is sent via unix sockets, it has to be serializable, so we also have to convert the method name to bytes.

Additionally a flaky test due to the unstable sorting of dictionary keys was made more robust.

Re-enable skipped tests.

We are on the journey to get the Bytes / String story solved for Python 2/3 compatibility.

This MP considers code in the landscape.lib.amp module which provides the basic functionality to execute methods on remote Python objects via amp. As such it provides an integral part of the system.

As all information are send via unix sockets, they have to be serializable, so we also have to convert the method name to bytes.

Additionally a flaky test due to the unstable sorting of dictionary keys was made more robust.

Finally, the skipped tests could be run without problems now.

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: TRIAL_ARGS=-j4 make check
Result: Success
Revno: 959
Branch: lp:~gocept/landscape-client/py3-amp
Jenkins: https://ci.lscape.net/job/latch-test-xenial/1470/

review: Approve (test results)

[Данило Шеган (danilo)]

Looks good, thanks for making the test more stable along the way. +1

review: Approve

[Free Ekanayaka (free.ekanayaka)]

Nice, +1

review: Approve

[lp:~gocept/landscape-client/py3-amp]

960. By Steffen Allner on 2017-03-13

Comply with coding style.

[Steffen Allner (sallner)]

I updated to landscape coding style.

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[Daniel Havlik (nilo)]

+1

review: Approve

[🤖 Landscape Builder (landscape-builder)]

Command: TRIAL_ARGS=-j4 make check
Result: Success
Revno: 960
Branch: lp:~gocept/landscape-client/py3-amp
Jenkins: https://ci.lscape.net/job/latch-test-xenial/3603/

review: Approve (test results)

=== modified file 'landscape/lib/amp.py'
--- landscape/lib/amp.py	2017-03-06 10:38:07 +0000
+++ landscape/lib/amp.py	2017-03-13 09:02:34 +0000
@@ -163,10 +163,13 @@
         if chunks is not None:
             # We got some L{MethodCallChunk}s before, this is the last.
             chunks.append(arguments)
-            arguments = "".join(chunks)
+            arguments = b"".join(chunks)
 
         args, kwargs = bpickle.loads(arguments)
 
+        # We encoded the method name in `send_method_call` and have to decode
+        # it here again.
+        method = method.decode("utf-8")
         if not method in self._methods:
             raise MethodCallError("Forbidden method '%s'" % method)
 
@@ -263,6 +266,8 @@
         """
         arguments = bpickle.dumps((args, kwargs))
         sequence = uuid4().int
+        # As we send the method name to remote, we need bytes.
+        method = method.encode("utf-8")
 
         # Split the given arguments in one or more chunks
         chunks = [arguments[i:i + self._chunk_size]

=== modified file 'landscape/lib/tests/test_amp.py'
--- landscape/lib/tests/test_amp.py	2017-01-10 09:31:30 +0000
+++ landscape/lib/tests/test_amp.py	2017-03-13 09:02:34 +0000
@@ -10,9 +10,6 @@
     MethodCallSender)
 from landscape.tests.helpers import LandscapeTest
 
-from unittest import skipIf
-from twisted.python.compat import _PY3
-
 
 class FakeTransport(object):
     """Accumulate written data into a list."""
@@ -207,12 +204,14 @@
         """
         Method arguments passed to a L{MethodCall} can be dictionaries.
         """
-        self.object.method = lambda d: "".join(d.keys()) * sum(d.values())
+        # Sort the keys to ensure stable test outcome.
+        self.object.method = lambda d: "".join(
+            sorted(d.keys()) * sum(d.values()))
         deferred = self.sender.send_method_call(method="method",
                                                 args=[{"foo": 1, "bar": 2}],
                                                 kwargs={})
         self.connection.flush()
-        self.assertEqual("foobarfoobarfoobar", self.successResultOf(deferred))
+        self.assertEqual("barfoobarfoobarfoo", self.successResultOf(deferred))
 
     def test_with_non_serializable_return_value(self):
         """
@@ -622,7 +621,6 @@
         self.client.stopTrying()
         connector.disconnect()
 
-    @skipIf(_PY3, 'Takes long with Python3, probably unclean Reactor')
     @inlineCallbacks
     def test_retry(self):
         """
@@ -644,7 +642,6 @@
         self.client.stopTrying()
         connector.disconnect()
 
-    @skipIf(_PY3, 'Takes long with Python3, probably unclean Reactor')
     @inlineCallbacks
     def test_retry_with_method_call_error(self):
         """
@@ -666,7 +663,6 @@
         self.client.stopTrying()
         connector.disconnect()
 
-    @skipIf(_PY3, 'Takes long with Python3, probably unclean Reactor')
     @inlineCallbacks
     def test_wb_retry_with_while_still_disconnected(self):
         """
@@ -710,7 +706,6 @@
         self.client.stopTrying()
         connector.disconnect()
 
-    @skipIf(_PY3, 'Takes long with Python3, probably unclean Reactor')
     @inlineCallbacks
     def test_retry_with_many_method_calls(self):
         """