Merge lp:~bjornt/landscape-client/apt-hardy-fixes into lp:~landscape/landscape-client/trunk

[Björn Tillenius]

Fix Apt related failures in hardy.

    * Ignore a FutureWarning from the apt module
    * Skip tests using AptFacade on hardy
    * Make hardy always use SmartFacade.

Hardy has a really old version of python-apt, and making AptFacade work
with it isn't trivial. So landscape client will keep using SmartFacade
on hardy.

There are still test failures on hardy, but this branch fixes the Apt
related ones. I'll look at the remaining failures next.

[Alberto Donato (ack)]

Looks good, +1!

review: Approve

[Free Ekanayaka (free.ekanayaka)]

Great, +1!

[1]

+try:
+ from apt.progress.text import AcquireProgress

I take there's no way to introspect the actual version number of the APT package?

review: Approve

[Björn Tillenius (bjornt)]

On Mon, Dec 12, 2011 at 09:09:23AM -0000, Free Ekanayaka wrote:
> Review: Approve
>
> Great, +1!
>
> [1]
>
> +try:
> + from apt.progress.text import AcquireProgress
>
> I take there's no way to introspect the actual version number of the
> APT package?

Not easily, no. Also, this is generally the pattern for dealing with
different versions.

--
Björn Tillenius | https://launchpad.net/~bjornt

=== modified file 'landscape/package/facade.py'
--- landscape/package/facade.py	2011-12-01 13:03:23 +0000
+++ landscape/package/facade.py	2011-12-09 10:30:30 +0000
@@ -9,10 +9,23 @@
 
 import smart
 
+# Importing apt throws a FutureWarning on hardy, that we don't want to
+# see.
+import warnings
+warnings.filterwarnings("ignore", module="apt", category=FutureWarning)
+del warnings
+
 import apt
 import apt_inst
 import apt_pkg
+
+has_new_enough_apt = True
 from aptsources.sourceslist import SourcesList
+try:
+    from apt.progress.text import AcquireProgress
+except ImportError:
+    AcquireProgress = object
+    has_new_enough_apt = False
 
 from landscape.lib.fs import append_file, create_file, read_file
 from landscape.package.skeleton import build_skeleton, build_skeleton_apt
@@ -41,7 +54,7 @@
     """Raised when channels fail to load."""
 
 
-class LandscapeAcquireProgress(apt.progress.text.AcquireProgress):
+class LandscapeAcquireProgress(AcquireProgress):
 
     def _winch(self, *dummy):
         """Override trying to get the column count of the buffer.

=== modified file 'landscape/package/taskhandler.py'
--- landscape/package/taskhandler.py	2011-11-15 09:06:01 +0000
+++ landscape/package/taskhandler.py	2011-12-09 10:30:30 +0000
@@ -266,11 +266,11 @@
     package_store = cls.package_store_class(config.store_filename)
     # Delay importing of the facades so that we don't
     # import Smart unless we need to.
-    if os.environ.get("USE_APT_FACADE"):
-        from landscape.package.facade import AptFacade
+    from landscape.package.facade import (
+        AptFacade, SmartFacade, has_new_enough_apt)
+    if os.environ.get("USE_APT_FACADE") and has_new_enough_apt:
         package_facade = AptFacade()
     else:
-        from landscape.package.facade import SmartFacade
         package_facade = SmartFacade()
 
     def finish():

=== modified file 'landscape/package/tests/test_changer.py'
--- landscape/package/tests/test_changer.py	2011-11-14 13:29:57 +0000
+++ landscape/package/tests/test_changer.py	2011-12-09 10:30:30 +0000
@@ -15,7 +15,7 @@
     POLICY_ALLOW_ALL_CHANGES)
 from landscape.package.store import PackageStore
 from landscape.package.facade import (
-    DependencyError, TransactionError, SmartError)
+    DependencyError, TransactionError, SmartError, has_new_enough_apt)
 from landscape.package.changer import (
     PackageChangerConfiguration, ChangePackagesResult)
 from landscape.tests.mocker import ANY
@@ -1040,6 +1040,9 @@
 
 class AptPackageChangerTest(LandscapeTest, PackageChangerTestMixin):
 
+    if not has_new_enough_apt:
+        skip = "Can't use AptFacade on hardy"
+
     helpers = [AptFacadeHelper, SimpleRepositoryHelper, BrokerServiceHelper]
 
     def setUp(self):

=== modified file 'landscape/package/tests/test_facade.py'
--- landscape/package/tests/test_facade.py	2011-12-06 15:06:55 +0000
+++ landscape/package/tests/test_facade.py	2011-12-09 10:30:30 +0000
@@ -21,7 +21,8 @@
 
 from landscape.lib.fs import read_file
 from landscape.package.facade import (
-    TransactionError, DependencyError, ChannelError, SmartError, AptFacade)
+    TransactionError, DependencyError, ChannelError, SmartError, AptFacade,
+    has_new_enough_apt)
 
 from landscape.tests.mocker import ANY
 from landscape.tests.helpers import LandscapeTest
@@ -54,6 +55,9 @@
 
 class AptFacadeTest(LandscapeTest):
 
+    if not has_new_enough_apt:
+        skip = "Can't use AptFacade on hardy"
+
     helpers = [AptFacadeHelper]
 
     def version_sortkey(self, version):

=== modified file 'landscape/package/tests/test_reporter.py'
--- landscape/package/tests/test_reporter.py	2011-12-06 10:53:50 +0000
+++ landscape/package/tests/test_reporter.py	2011-12-09 10:30:30 +0000
@@ -17,7 +17,7 @@
     PackageReporter, HASH_ID_REQUEST_TIMEOUT, main, find_reporter_command,
     PackageReporterConfiguration, FakeGlobalReporter, FakeReporter)
 from landscape.package import reporter
-from landscape.package.facade import AptFacade
+from landscape.package.facade import AptFacade, has_new_enough_apt
 from landscape.package.tests.helpers import (
     SmartFacadeHelper, AptFacadeHelper, SimpleRepositoryHelper,
     HASH1, HASH2, HASH3, PKGNAME1)
@@ -1620,6 +1620,9 @@
 
 class PackageReporterAptTest(LandscapeTest, PackageReporterTestMixin):
 
+    if not has_new_enough_apt:
+        skip = "Can't use AptFacade on hardy"
+
     helpers = [AptFacadeHelper, SimpleRepositoryHelper, BrokerServiceHelper]
 
     Facade = AptFacade
@@ -1950,6 +1953,9 @@
 class GlobalPackageReporterAptTest(LandscapeTest,
                                    GlobalPackageReporterTestMixin):
 
+    if not has_new_enough_apt:
+        skip = "Can't use AptFacade on hardy"
+
     helpers = [AptFacadeHelper, SimpleRepositoryHelper, BrokerServiceHelper]
 
     def setUp(self):

=== modified file 'landscape/package/tests/test_skeleton.py'
--- landscape/package/tests/test_skeleton.py	2011-10-14 08:38:13 +0000
+++ landscape/package/tests/test_skeleton.py	2011-12-09 10:30:30 +0000
@@ -5,6 +5,7 @@
 from landscape.package.interface import (
     install_landscape_interface, uninstall_landscape_interface)
 
+from landscape.package.facade import has_new_enough_apt
 from landscape.package.skeleton import (
     build_skeleton, PackageTypeError, build_skeleton_apt, DEB_PROVIDES,
     DEB_NAME_PROVIDES, DEB_REQUIRES, DEB_OR_REQUIRES, DEB_UPGRADES,
@@ -287,6 +288,9 @@
 class SkeletonAptTest(LandscapeTest, SkeletonTestMixin):
     """C{PackageSkeleton} tests for apt packages."""
 
+    if not has_new_enough_apt:
+        skip = "Can't use AptFacade on hardy"
+
     helpers = [AptFacadeHelper, SkeletonTestHelper]
 
     def setUp(self):

=== modified file 'landscape/package/tests/test_taskhandler.py'
--- landscape/package/tests/test_taskhandler.py	2011-11-17 08:46:12 +0000
+++ landscape/package/tests/test_taskhandler.py	2011-12-09 10:30:30 +0000
@@ -9,6 +9,7 @@
 from landscape.package.taskhandler import (
     PackageTaskHandlerConfiguration, PackageTaskHandler, run_task_handler,
     LazyRemoteBroker)
+from landscape.package import facade as facade_module
 from landscape.package.facade import AptFacade, SmartFacade
 from landscape.package.store import HashIdStore, PackageStore
 from landscape.package.tests.helpers import AptFacadeHelper, SmartFacadeHelper
@@ -419,14 +420,16 @@
         result = run_task_handler(HandlerMock, ["-c", self.config_filename])
         return result.addCallback(assert_task_handler)
 
-    def test_run_task_handler_use_apt_facade(self):
+    def test_run_task_handler_use_apt_facade_new_apt(self):
         """
         The L{run_task_handler} creates an AptFacade instead of
-        SmartFacade, if the USE_APT_FACADE environment variable is set.
+        SmartFacade, if the USE_APT_FACADE environment variable is set
+        and C{python-apt} is new enough.
         """
 
         HandlerMock, handler_args = self._mock_run_task_handler()
         os.environ["USE_APT_FACADE"] = "1"
+        self._set_new_enough_apt(True)
 
         def assert_task_handler(ignored):
 
@@ -447,6 +450,49 @@
         result = run_task_handler(HandlerMock, ["-c", self.config_filename])
         return result.addCallback(assert_task_handler)
 
+    def _set_new_enough_apt(self, value):
+        """Override landscape.package.facade.has_new_enough_apt.
+
+        The previous value of that attribute is replaced when the test
+        is finished.
+        """
+
+        def reset_new_enough_apt():
+            facade_module.has_new_enough_apt = old_has_new_enough_apt
+
+        old_has_new_enough_apt = facade_module.has_new_enough_apt
+        facade_module.has_new_enough_apt = value
+        self.addCleanup(reset_new_enough_apt)
+
+
+    def test_run_task_handler_use_apt_facade_old_apt(self):
+        """
+        If the C{python-apt} module isn't new enough,
+        C{run_task_handler} will create a C{SmartFacade}, even when
+        USE_APT_FACADE is set, since C{AptFacade} doesn't work with old
+        versions of C{python-apt}.
+
+        Hardy has a too old version of C{python-apt}, but lucid and
+        onwards should have a new enough version.
+        """
+
+        HandlerMock, handler_args = self._mock_run_task_handler()
+        os.environ["USE_APT_FACADE"] = "1"
+        self._set_new_enough_apt(False)
+
+        def assert_task_handler(ignored):
+
+            store, facade, broker, config = handler_args
+
+            try:
+                self.assertEqual(type(facade), SmartFacade)
+            finally:
+                # Put reactor back in place before returning.
+                self.mocker.reset()
+
+        result = run_task_handler(HandlerMock, ["-c", self.config_filename])
+        return result.addCallback(assert_task_handler)
+
     def test_run_task_handler_when_already_locked(self):
 
         lock_path(os.path.join(self.data_path, "package", "default.lock"))