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

[Steffen Allner]

This is the Py3 port of landscape.monitor.swiftusage.

We make a few bytes/unicode fixes and address a test ordering issue.

This patch also completes the fundamental Py3 port of the Landscape client. So all tests are now run under both Python 2 and 3, rather than just the selection of tests that had been ported thus far. Further work remains to address remaining (untested) Python 3 incompatibilities.

This MP is the last one to port landscape.monitor to Python 3. In l.m.swiftusage an encoding was done, probably to ensure bytes as filename for the backend in the (Rooted)Persist. As these are normal files on the filesystem it should not be a problem to have it as unicodes.

Additionally the whole test suite is now running on Python 2 and 3 at the same time.
Two test in test_swiftusage are skipped because python-swift is not yet ported to Python 3.

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: TRIAL_ARGS=-j4 make ci-check
Result: Fail
Revno: 1002
Branch: lp:~gocept/landscape-client/py3-monitor-swiftusage
Jenkins: https://ci.lscape.net/job/latch-test-xenial/3824/

review: Needs Fixing (test results)

[lp:~gocept/landscape-client/py3-monitor-swiftusage]

1003. By Steffen Allner on 2017-03-30

Stabilize order by sorting.

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

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

review: Approve (test results)

[Daniel Havlik (nilo)]

+1

review: Approve

[Eric Snow (ericsnowcurrently)]

LGTM

review: Approve

[Eric Snow (ericsnowcurrently)]

...and nice work on getting all the landscape-client tests passing under Python 3!!!

=== modified file 'Makefile'
--- Makefile	2017-03-26 04:07:33 +0000
+++ Makefile	2017-03-30 14:14:06 +0000
@@ -5,9 +5,8 @@
 TRIAL_ARGS ?=
 TEST_COMMAND_PY2 = trial --unclean-warnings $(TRIAL_ARGS) landscape
 TEST_COMMAND_PY3 = trial3 --unclean-warnings $(TRIAL_ARGS) landscape
-READY_FILE := py3_ready_tests
-PY3_READY := `cat $(READY_FILE)`
-TEST_COMMAND_PY3_READY = TRIAL_ARGS= trial3 --unclean-warnings $(PY3_READY)
+# trial3 does not support threading via `-j` at the moment
+TEST_COMMAND_PY3_READY = TRIAL_ARGS= trial3 --unclean-warnings landscape
 UBUNTU_RELEASE := $(shell lsb_release -cs)
 # version in the code is authoritative
 # Use := here, not =, it's really important, otherwise UPSTREAM_VERSION

=== modified file 'landscape/lib/persist.py'
--- landscape/lib/persist.py	2017-03-17 09:26:45 +0000
+++ landscape/lib/persist.py	2017-03-30 14:14:06 +0000
@@ -23,6 +23,8 @@
 import copy
 import re
 
+from twisted.python.compat import StringType  # Py2: basestring, Py3: str
+
 
 __all__ = ["Persist", "PickleBackend", "BPickleBackend",
            "path_string_to_tuple", "path_tuple_to_string", "RootedPersist",
@@ -201,7 +203,7 @@
         return newobj
 
     def _getvalue(self, path, soft=False, hard=False, weak=False):
-        if type(path) is str:
+        if isinstance(path, StringType):
             path = path_string_to_tuple(path)
         marker = NOTHING
         if soft:
@@ -247,7 +249,7 @@
 
     def set(self, path, value, soft=False, weak=False):
         assert path
-        if type(path) is str:
+        if isinstance(path, StringType):
             path = path_string_to_tuple(path)
         if soft:
             map = self._softmap
@@ -261,7 +263,7 @@
 
     def add(self, path, value, unique=False, soft=False, weak=False):
         assert path
-        if type(path) is str:
+        if isinstance(path, StringType):
             path = path_string_to_tuple(path)
         if soft:
             map = self._softmap
@@ -280,7 +282,7 @@
 
     def remove(self, path, value=NOTHING, soft=False, weak=False):
         assert path
-        if type(path) is str:
+        if isinstance(path, StringType):
             path = path_string_to_tuple(path)
         if soft:
             map = self._softmap
@@ -318,9 +320,9 @@
     def move(self, oldpath, newpath, soft=False, weak=False):
         if not (soft or weak):
             self.assert_writable()
-        if type(oldpath) is str:
+        if isinstance(oldpath, StringType):
             oldpath = path_string_to_tuple(oldpath)
-        if type(newpath) is str:
+        if isinstance(newpath, StringType):
             newpath = path_string_to_tuple(newpath)
         result = False
         marker = NOTHING
@@ -358,7 +360,7 @@
             will be used as root of this L{RootedPersist}.
         """
         self.parent = parent
-        if type(root) is str:
+        if isinstance(root, StringType):
             self.root = path_string_to_tuple(root)
         else:
             self.root = root
@@ -370,45 +372,45 @@
         self.parent.assert_writable()
 
     def has(self, path, value=NOTHING, soft=False, hard=False, weak=False):
-        if type(path) is str:
+        if isinstance(path, StringType):
             path = path_string_to_tuple(path)
         return self.parent.has(self.root + path, value, soft, hard, weak)
 
     def keys(self, path, soft=False, hard=False, weak=False):
-        if type(path) is str:
+        if isinstance(path, StringType):
             path = path_string_to_tuple(path)
         return self.parent.keys(self.root + path, soft, hard, weak)
 
     def get(self, path, default=None, soft=False, hard=False, weak=False):
-        if type(path) is str:
+        if isinstance(path, StringType):
             path = path_string_to_tuple(path)
         return self.parent.get(self.root + path, default, soft, hard, weak)
 
     def set(self, path, value, soft=False, weak=False):
-        if type(path) is str:
+        if isinstance(path, StringType):
             path = path_string_to_tuple(path)
         return self.parent.set(self.root + path, value, soft, weak)
 
     def add(self, path, value, unique=False, soft=False, weak=False):
-        if type(path) is str:
+        if isinstance(path, StringType):
             path = path_string_to_tuple(path)
         return self.parent.add(self.root + path, value, unique, soft, weak)
 
     def remove(self, path, value=NOTHING, soft=False, weak=False):
-        if type(path) is str:
+        if isinstance(path, StringType):
             path = path_string_to_tuple(path)
         return self.parent.remove(self.root + path, value, soft, weak)
 
     def move(self, oldpath, newpath, soft=False, weak=False):
-        if type(oldpath) is str:
+        if isinstance(oldpath, StringType):
             oldpath = path_string_to_tuple(oldpath)
-        if type(newpath) is str:
+        if isinstance(newpath, StringType):
             newpath = path_string_to_tuple(newpath)
         return self.parent.move(self.root + oldpath, self.root + newpath,
                                 soft, weak)
 
     def root_at(self, path):
-        if type(path) is str:
+        if isinstance(path, StringType):
             path = path_string_to_tuple(path)
         return self.parent.root_at(self.root + path)
 

=== modified file 'landscape/monitor/swiftusage.py'
--- landscape/monitor/swiftusage.py	2016-07-20 20:10:32 +0000
+++ landscape/monitor/swiftusage.py	2017-03-30 14:14:06 +0000
@@ -135,7 +135,7 @@
             if not usage["mounted"]:
                 continue
 
-            device = usage["device"].encode("utf-8")
+            device = usage["device"]
             devices.add(device)
 
             step_values = []

=== modified file 'landscape/monitor/tests/test_swiftusage.py'
--- landscape/monitor/tests/test_swiftusage.py	2016-07-20 20:10:32 +0000
+++ landscape/monitor/tests/test_swiftusage.py	2017-03-30 14:14:06 +0000
@@ -186,7 +186,8 @@
             [(30, "vdb", 100000, 80000, 20000),
              (30, "vdd", 200000, 10000, 190000)],
             self.plugin._swift_usage_points)
-        self.assertEqual(["vdb", "vdd"], self.plugin._persist.get("devices"))
+        self.assertEqual(
+            ["vdb", "vdd"], sorted(self.plugin._persist.get("devices")))
         self.assertNotIn("vdc", self.plugin._persist.get("usage"))
 
     def test_message_remove_disappeared_devices(self):

=== removed file 'py3_ready_tests'
--- py3_ready_tests	2017-03-30 14:14:06 +0000
+++ py3_ready_tests	1970-01-01 00:00:00 +0000
@@ -1,19 +0,0 @@
-landscape.lib.tests
-landscape.sysinfo.tests
-landscape.user.tests
-landscape.broker.tests
-landscape.package.tests
-landscape.manager.tests
-landscape.tests
-
-
-landscape.monitor.tests.test_activeprocessinfo
-landscape.monitor.tests.test_computeruptime
-landscape.monitor.tests.test_mountinfo
-landscape.monitor.tests.test_computerinfo
-landscape.monitor.tests.test_loadaverage
-landscape.monitor.tests.test_networkdevice
-landscape.monitor.tests.test_networkactivity
-landscape.monitor.tests.test_packagemonitor
-landscape.monitor.tests.test_aptpreferences
-landscape.monitor.tests.test_processorinfo