Merge lp:~free.ekanayaka/landscape-client/create-package-lock into lp:~landscape/landscape-client/trunk

[Free Ekanayaka]

[lp:~free.ekanayaka/landscape-client/create-package-lock]

202. By Free Ekanayaka on 2010-02-01

Fix wrong status and result-code message field values.

[Thomas Herve (therve)]

[1]
+ The L{PackageManager.handle_change_package_locks} method is registered
+ has handler for messages of type C{"change-package-locks"}, and queues

Typo: as

Great small branch, +1!

review: Approve

[Free Ekanayaka (free.ekanayaka)]

Thanks Thomas! [1] got fixed.

[Sidnei da Silva (sidnei)]

Great branch. +1! Also the first one I use the new 'bzr ls-lint' command on. :)

There's a little lint here and there:

sidnei@sidnei-laptop:~/src/landscape/client/review/create-package-lock$ bzr ls-lint

= Landscape lint =

Linting changed files:
  landscape/__init__.py
  landscape/manager/packagemanager.py
  landscape/manager/tests/test_packagemanager.py
  landscape/package/changer.py
  landscape/package/facade.py
  landscape/package/tests/helpers.py
  landscape/package/tests/test_changer.py
  landscape/package/tests/test_facade.py

== lint ==

=== jslint ===

(No files to lint)

=== pep8 ===

landscape/package/tests/test_facade.py:39:9: E301 expected 1 blank line, found 0
landscape/package/tests/test_facade.py:39:34: E701 multiple statements on one line (colon)
landscape/package/tests/test_facade.py:56:9: E301 expected 1 blank line, found 0
landscape/package/tests/test_facade.py:56:34: E701 multiple statements on one line (colon)
landscape/package/tests/test_facade.py:347:9: E301 expected 1 blank line, found 0
landscape/package/tests/test_facade.py:396:9: E301 expected 1 blank line, found 0
landscape/package/tests/test_facade.py:396:34: E701 multiple statements on one line (colon)
landscape/package/tests/test_facade.py:456:13: E301 expected 1 blank line, found 0
landscape/package/tests/test_changer.py:38:80: E501 line too long (88 characters)
landscape/package/tests/test_changer.py:49:9: E301 expected 1 blank line, found 0
landscape/package/tests/test_changer.py:56:9: E301 expected 1 blank line, found 0
landscape/package/tests/test_changer.py:66:9: E301 expected 1 blank line, found 0
landscape/package/tests/test_changer.py:75:9: E301 expected 1 blank line, found 0
landscape/package/tests/test_changer.py:117:9: E301 expected 1 blank line, found 0
landscape/package/tests/test_changer.py:366:1: W291 trailing whitespace
landscape/package/tests/test_changer.py:570:5: E301 expected 1 blank line, found 2
landscape/package/tests/test_changer.py:615:1: W291 trailing whitespace
landscape/package/tests/test_changer.py:620:1: W291 trailing whitespace
landscape/package/tests/helpers.py:146:80: E501 line too long (80 characters)
landscape/package/tests/helpers.py:149:80: E501 line too long (81 characters)
landscape/package/tests/helpers.py:183:80: E501 line too long (80 characters)
landscape/package/tests/helpers.py:194:80: E501 line too long (132 characters)
landscape/package/tests/helpers.py:211:80: E501 line too long (132 characters)
landscape/package/tests/helpers.py:228:80: E501 line too long (94 characters)
landscape/package/tests/helpers.py:247:80: E501 line too long (80 characters)
landscape/package/tests/helpers.py:273:80: E501 line too long (91 characters)
landscape/package/tests/helpers.py:286:80: E501 line too long (80 characters)
landscape/package/tests/helpers.py:291:80: E501 line too long (80 characters)
landscape/package/tests/helpers.py:303:80: E501 line too long (91 characters)
landscape/package/tests/helpers.py:316:80: E501 line too long (80 characters)
landscape/package/tests/helpers.py:321:...

review: Approve

[lp:~free.ekanayaka/landscape-client/create-package-lock]

203. By Free Ekanayaka on 2010-02-02

Fix typo (Thomas [1])

204. By Free Ekanayaka on 2010-02-02

Make tests lint clean(er) (Sidnei)

[Free Ekanayaka (free.ekanayaka)]

Thanks Sidnei, I didn't check out ls-lint yet, but will do soon! I think I've fixed most of the warnings, however there are a few ones in helpers.py that are a bit hard, because there is some hard-coded text. Maybe will do it in another pass.

=== modified file 'landscape/__init__.py'
--- landscape/__init__.py	2010-01-27 16:49:11 +0000
+++ landscape/__init__.py	2010-02-02 13:46:11 +0000
@@ -14,7 +14,15 @@
 
 # The "client-api" field of outgoing messages will be set to this value, and
 # used by the server to know which schema do the message types accepted by the
-# client support. Bump it when the schema of an accepted message type changes.
+# client support. Bump it when the schema of an accepted message type changes
+# and update the changelog below as needed.
+#
+# Changelog:
+#
+# 3.3:
+#  * Add "binaries" field to "change-packages"
+#  * Add new "change-package-locks" client accepted message type.
+#
 CLIENT_API = "3.3"
 
 from twisted.python import util

=== modified file 'landscape/manager/packagemanager.py'
--- landscape/manager/packagemanager.py	2009-10-28 10:11:13 +0000
+++ landscape/manager/packagemanager.py	2010-02-02 13:46:11 +0000
@@ -26,6 +26,8 @@
 
         registry.register_message("change-packages",
                                   self.handle_change_packages)
+        registry.register_message("change-package-locks",
+                                  self.handle_change_package_locks)
         registry.register_message("release-upgrade",
                                   self.handle_release_upgrade)
 
@@ -39,6 +41,9 @@
     def handle_change_packages(self, message):
         return self._handle(PackageChanger, message)
 
+    def handle_change_package_locks(self, message):
+        return self._handle(PackageChanger, message)
+
     def handle_release_upgrade(self, message):
         return self._handle(ReleaseUpgrader, message)
 

=== modified file 'landscape/manager/tests/test_packagemanager.py'
--- landscape/manager/tests/test_packagemanager.py	2009-10-30 15:09:34 +0000
+++ landscape/manager/tests/test_packagemanager.py	2010-02-02 13:46:11 +0000
@@ -144,7 +144,6 @@
         self.mocker.replay()
 
         message = {"type": "change-packages"}
-        service = self.broker_service
         self.manager.dispatch_message(message)
         task = self.package_store.get_next_task("changer")
         self.assertTrue(task)
@@ -316,3 +315,21 @@
             os.chmod(dir, 0766)
 
         return result.addCallback(got_result)
+
+    def test_change_package_locks_handling(self):
+        """
+        The L{PackageManager.handle_change_package_locks} method is registered
+        as handler for messages of type C{"change-package-locks"}, and queues
+        a package-changer task in the appropriate queue.
+        """
+        self.manager.add(self.package_manager)
+
+        package_manager_mock = self.mocker.patch(self.package_manager)
+        package_manager_mock.spawn_handler(PackageChanger)
+        self.mocker.replay()
+
+        message = {"type": "change-package-locks"}
+        self.manager.dispatch_message(message)
+        task = self.package_store.get_next_task("changer")
+        self.assertTrue(task)
+        self.assertEquals(task.data, message)

=== modified file 'landscape/package/changer.py'
--- landscape/package/changer.py	2010-01-20 10:05:27 +0000
+++ landscape/package/changer.py	2010-02-02 13:46:11 +0000
@@ -11,6 +11,7 @@
 from landscape.package.reporter import find_reporter_command
 from landscape.package.taskhandler import (
     PackageTaskHandler, PackageTaskHandlerConfiguration, run_task_handler)
+from landscape.manager.manager import SUCCEEDED
 
 
 SUCCESS_RESULT = 1
@@ -91,6 +92,8 @@
         if message["type"] == "change-packages":
             result = self._handle_change_packages(message)
             return result.addErrback(self._check_expired_unknown_data, task)
+        if message["type"] == "change-package-locks":
+            return self._handle_change_package_locks(message)
 
     def _warn_about_unknown_data(self, failure):
         failure.trap(UnknownPackageData)
@@ -209,6 +212,28 @@
                      "exchange urgently.")
         return self._broker.send_message(message, True)
 
+    def _handle_change_package_locks(self, message):
+        """Handle a C{change-package-locks} message.
+
+        Create and delete package locks as requested by the given C{message}.
+        """
+
+        for lock in message.get("create", ()):
+            self._facade.set_package_lock(*lock)
+        for lock in message.get("delete", ()):
+            self._facade.remove_package_lock(*lock)
+        self._facade.save_config()
+
+        response = {"type": "operation-result",
+                    "operation-id": message.get("operation-id"),
+                    "status": SUCCEEDED,
+                    "result-text": "Package locks successfully changed.",
+                    "result-code": 0}
+
+        logging.info("Queuing message with change package locks results to "
+                     "exchange urgently.")
+        return self._broker.send_message(response, True)
+
     @staticmethod
     def find_command():
         return find_changer_command()

=== modified file 'landscape/package/facade.py'
--- landscape/package/facade.py	2009-11-26 02:54:44 +0000
+++ landscape/package/facade.py	2010-02-02 13:46:11 +0000
@@ -351,9 +351,14 @@
         self._get_ctrl()
         smart.pkgconf.setFlag("lock", name, relation, version)
 
-    def remove_package_lock(self, name, relation=(), version=()):
+    def remove_package_lock(self, name, relation=None, version=None):
         """Remove a package lock."""
         self._validate_lock_condition(relation, version)
         self._get_ctrl()
         smart.pkgconf.clearFlag("lock", name=name, relation=relation,
                                 version=version)
+
+    def save_config(self):
+        """Flush the current smart configuration to disk."""
+        control = self._get_ctrl()
+        control.saveSysConf()

=== modified file 'landscape/package/tests/helpers.py'
--- landscape/package/tests/helpers.py	2010-01-20 10:00:02 +0000
+++ landscape/package/tests/helpers.py	2010-02-02 13:46:11 +0000
@@ -8,6 +8,7 @@
 
     def set_up(self, test_case):
         test_case.smart_dir = test_case.makeDir()
+        test_case.smart_config = test_case.makeFile("")
         test_case.repository_dir = test_case.makeDir()
         create_simple_repository(test_case.repository_dir)
 
@@ -31,7 +32,8 @@
                 self.add_channel_deb_dir(test_case.repository_dir)
 
         test_case.Facade = Facade
-        test_case.facade = Facade({"datadir": test_case.smart_dir})
+        test_case.facade = Facade({"datadir": test_case.smart_dir,
+                                   "configfile": test_case.smart_config})
 
 
 PKGNAME1 = "name1_version1-release1_all.deb"

=== modified file 'landscape/package/tests/test_changer.py'
--- landscape/package/tests/test_changer.py	2010-01-20 10:05:27 +0000
+++ landscape/package/tests/test_changer.py	2010-02-02 13:46:11 +0000
@@ -19,8 +19,8 @@
 from landscape.tests.helpers import (
     LandscapeIsolatedTest, RemoteBrokerHelper)
 from landscape.package.tests.helpers import (
-    SmartFacadeHelper, HASH1, HASH2, HASH3, PKGDEB1, PKGDEB2, PKGDEB3,
-    PKGNAME2)
+    SmartFacadeHelper, HASH1, HASH2, HASH3, PKGDEB1, PKGDEB2, PKGNAME2)
+from landscape.manager.manager import SUCCEEDED
 
 
 class PackageChangerTest(LandscapeIsolatedTest):
@@ -35,16 +35,18 @@
         self.config.data_path = self.makeDir()
         os.mkdir(self.config.package_directory)
         os.mkdir(self.config.binaries_path)
-        self.changer = PackageChanger(self.store, self.facade, self.remote, self.config)
-
+        self.changer = PackageChanger(self.store, self.facade, self.remote,
+                                      self.config)
         service = self.broker_service
-        service.message_store.set_accepted_types(["change-packages-result"])
+        service.message_store.set_accepted_types(["change-packages-result",
+                                                  "operation-result"])
 
     def get_pending_messages(self):
         return self.broker_service.message_store.get_pending_messages()
 
     def set_pkg1_installed(self):
         previous = self.Facade.channels_reloaded
+
         def callback(self):
             previous(self)
             self.get_packages_by_name("name1")[0].installed = True
@@ -52,6 +54,7 @@
 
     def set_pkg2_upgrades_pkg1(self):
         previous = self.Facade.channels_reloaded
+
         def callback(self):
             from smart.backends.deb.base import DebUpgrades
             previous(self)
@@ -62,6 +65,7 @@
 
     def set_pkg2_satisfied(self):
         previous = self.Facade.channels_reloaded
+
         def callback(self):
             previous(self)
             pkg2 = self.get_packages_by_name("name2")[0]
@@ -71,6 +75,7 @@
 
     def set_pkg1_and_pkg2_satisfied(self):
         previous = self.Facade.channels_reloaded
+
         def callback(self):
             previous(self)
 
@@ -113,6 +118,7 @@
         # result of our previous message, which got *postponed*.
         self.store.set_hash_ids({HASH2: 2})
         result = self.changer.handle_tasks()
+
         def got_result(result):
             self.assertMessages(self.get_pending_messages(),
                                 [{"must-install": [2],
@@ -362,7 +368,7 @@
 
     def test_successful_operation(self):
         """Simulate a *very* successful operation.
-        
+
         We'll do that by hacking perform_changes(), and returning our
         *very* successful operation result.
         """
@@ -565,7 +571,6 @@
                                         "operation-id": 123})
         return self.changer.run()
 
-
     def test_run(self):
         changer_mock = self.mocker.patch(self.changer)
 
@@ -611,7 +616,7 @@
 
     def test_main(self):
         self.mocker.order()
-        
+
         run_task_handler = self.mocker.replace("landscape.package.taskhandler"
                                                ".run_task_handler",
                                                passthrough=False)
@@ -752,3 +757,80 @@
         self.makeFile(basename=existing_deb_path, content="foo")
         self.changer._create_deb_dir_channel([])
         self.assertFalse(os.path.exists(existing_deb_path))
+
+    def test_change_package_locks(self):
+        """
+        The L{PackageChanger.handle_tasks} method appropriately creates and
+        deletes package locks as requested by the C{change-package-locks}
+        message.
+        """
+        self.facade.set_package_lock("bar")
+        self.store.add_task("changer", {"type": "change-package-locks",
+                                        "create": [("foo", ">=", "1.0")],
+                                        "delete": [("bar", None, None)],
+                                        "operation-id": 123})
+
+        def assert_result(result):
+            self.facade.deinit()
+            self.assertEquals(self.facade.get_package_locks(),
+                              [("foo", ">=", "1.0")])
+            self.assertIn("Queuing message with change package locks results "
+                          "to exchange urgently.", self.logfile.getvalue())
+            self.assertMessages(self.get_pending_messages(),
+                                [{"type": "operation-result",
+                                  "operation-id": 123,
+                                  "status": SUCCEEDED,
+                                  "result-text": "Package locks successfully"
+                                                 " changed.",
+                                  "result-code": 0}])
+
+        result = self.changer.handle_tasks()
+        return result.addCallback(assert_result)
+
+    def test_change_package_locks_create_with_already_existing(self):
+        """
+        The L{PackageChanger.handle_tasks} method gracefully handles requests
+        for creating package locks that already exist.
+        """
+        self.facade.set_package_lock("foo")
+        self.store.add_task("changer", {"type": "change-package-locks",
+                                        "create": [("foo", None, None)],
+                                        "operation-id": 123})
+
+        def assert_result(result):
+            self.facade.deinit()
+            self.assertEquals(self.facade.get_package_locks(),
+                              [("foo", "", "")])
+            self.assertMessages(self.get_pending_messages(),
+                                [{"type": "operation-result",
+                                  "operation-id": 123,
+                                  "status": SUCCEEDED,
+                                  "result-text": "Package locks successfully"
+                                                 " changed.",
+                                  "result-code": 0}])
+
+        result = self.changer.handle_tasks()
+        return result.addCallback(assert_result)
+
+    def test_change_package_locks_delete_without_already_existing(self):
+        """
+        The L{PackageChanger.handle_tasks} method gracefully handles requests
+        for deleting package locks that don't exist.
+        """
+        self.store.add_task("changer", {"type": "change-package-locks",
+                                        "delete": [("foo", ">=", "1.0")],
+                                        "operation-id": 123})
+
+        def assert_result(result):
+            self.facade.deinit()
+            self.assertEquals(self.facade.get_package_locks(), [])
+            self.assertMessages(self.get_pending_messages(),
+                                [{"type": "operation-result",
+                                  "operation-id": 123,
+                                  "status": SUCCEEDED,
+                                  "result-text": "Package locks successfully"
+                                                 " changed.",
+                                  "result-code": 0}])
+
+        result = self.changer.handle_tasks()
+        return result.addCallback(assert_result)

=== modified file 'landscape/package/tests/test_facade.py'
--- landscape/package/tests/test_facade.py	2009-11-25 09:48:00 +0000
+++ landscape/package/tests/test_facade.py	2010-02-02 13:46:11 +0000
@@ -36,7 +36,10 @@
     def test_get_packages_wont_return_non_debian_packages(self):
         self.facade.reload_channels()
         ctrl_mock = self.mocker.patch(Control)
-        class StubPackage(object): pass
+
+        class StubPackage(object):
+            pass
+
         cache_mock = ctrl_mock.getCache()
         cache_mock.getPackages()
         self.mocker.result([StubPackage(), StubPackage()])
@@ -53,7 +56,10 @@
     def test_get_packages_by_name_wont_return_non_debian_packages(self):
         self.facade.reload_channels()
         ctrl_mock = self.mocker.patch(Control)
-        class StubPackage(object): pass
+
+        class StubPackage(object):
+            pass
+
         cache_mock = ctrl_mock.getCache()
         cache_mock.getPackages("name")
         self.mocker.result([StubPackage(), StubPackage()])
@@ -116,7 +122,7 @@
         # Forcibly change the mtime of our repository, so that Smart
         # will consider it as changed (if the change is inside the
         # same second the directory's mtime will be the same)
-        mtime = int(time.time()+1)
+        mtime = int(time.time() + 1)
         os.utime(self.repository_dir, (mtime, mtime))
 
         # Reload channels.
@@ -344,6 +350,7 @@
         self.facade.mark_install(pkg)
 
         environ = []
+
         def check_environ(self, argv, output):
             environ.append(os.environ.get("DEBIAN_FRONTEND"))
             environ.append(os.environ.get("APT_LISTCHANGES_FRONTEND"))
@@ -393,7 +400,10 @@
         self.facade.deinit()
 
     def test_reload_channels_wont_consider_non_debian_packages(self):
-        class StubPackage(object): pass
+
+        class StubPackage(object):
+            pass
+
         pkg = StubPackage()
 
         ctrl_mock = self.mocker.patch(Control)
@@ -453,6 +463,7 @@
         def do_test():
             result = getProcessOutputAndValue("/usr/bin/dpkg",
                                               ("--print-architecture",))
+
             def callback((out, err, code)):
                 self.assertEquals(self.facade.get_arch(), out.strip())
             result.addCallback(callback)
@@ -605,3 +616,13 @@
         self.facade.set_package_lock("name1", "<", "version1")
         self.facade.remove_package_lock("name1", "<", "version1")
         self.assertEquals(self.facade.get_locked_packages(), [])
+
+    def test_save_config(self):
+        """
+        It is possible to lock a package by simply specifying its name.
+        """
+        self.facade.set_package_lock("python", "=>", "2.5")
+        self.facade.save_config()
+        self.facade.deinit()
+        self.assertEquals(self.facade.get_package_locks(),
+                          [("python", "=>", "2.5")])