Merge lp:~james-w/udd/binary-scanning-series into lp:udd

[James Westby]

Fix the API expectations of iter_published_binaries to match the API that is passed from the caller.

Hi,

We're getting crashes for list-packages in binary scanning mode, because
of an API mismatch.

This fixes that up, as well as tweaking the docstring to make it clear
what the API is.

Thanks,

James

[Jonathan Lange (jml)]

LGTM. Thanks.

review: Approve

=== modified file 'udd/icommon.py'
--- udd/icommon.py	2012-08-09 15:45:46 +0000
+++ udd/icommon.py	2012-08-11 16:10:27 +0000
@@ -395,6 +395,39 @@
             distro_arch_series=distro_arch_series, **filters))
 
 
+def _lp_get_series(distribution, name_or_version):
+    return lpapi.lp_call(distribution.getSeries,
+            name_or_version=name_or_version)
+
+
+def _iter_published_binaries_for_series(distribution, series,
+        status='Published', since=None):
+    """Iterate over published binary packages for a particular series.
+
+    :param distribution: A Launchpad ``Distribution`` object. e.g. ubuntu.
+    :param series: A Launchpad ``DistroSeries`` object, e.g. 'precise' in
+        'ubuntu.
+    :param status: One of 'Pending', 'Published', 'Superseded', 'Deleted'
+        or 'Obsolete'.  Defaults to 'Published'.
+    :param since: If provided, only find packages published since that date.
+    :return: An iterator over source package publishing history objects.
+        See https://launchpad.net/+apidoc/devel.html#binary_package_publishing_history
+        for more details.
+    """
+    filters = {'status': status}
+    if since:
+        filters['created_since_date'] = since
+    archive = distribution.main_archive
+    collections = []
+    # There's no way to query getPublishedBinaries by distro_series, so we
+    # have to search by distro_arch_series and combine the results.
+    # <https://bugs.launchpad.net/launchpad/+bug/904670>.
+    for distro_arch_series in series.architectures:
+        collections.append(
+            _lp_get_published_binaries(archive, distro_arch_series, **filters))
+    return chain(*collections)
+
+
 def iter_published_binaries(distribution, status='Published', since=None,
                             series=None):
     """Iterate over published binary packages.
@@ -403,15 +436,12 @@
     :param status: One of 'Pending', 'Published', 'Superseded', 'Deleted'
         or 'Obsolete'.  Defaults to 'Published'.
     :param since: If provided, only find packages published since that date.
-    :param series: If provided, only find packages published to that series.
+    :param series: The name or version of the distribution series to restrict
+        the search to. e.g. 'oneiric'
     :return: An iterator over source package publishing history objects.
         See https://launchpad.net/+apidoc/devel.html#binary_package_publishing_history
         for more details.
     """
-    filters = {'status': status}
-    if since:
-        filters['created_since_date'] = since
-    archive = distribution.main_archive
     if series is None:
         # If series is None, then a single, unadorned getPublishedBinaries
         # query will time out (bug #904344).  Instead, break down the query
@@ -419,16 +449,11 @@
         collections = []
         for s in distribution.series:
             collections.append(
-                iter_published_binaries(distribution, status, since, s))
+                _iter_published_binaries_for_series(distribution, s, status, since))
         return chain(*collections)
-    collections = []
-    # There's no way to query getPublishedBinaries by distro_series, so we
-    # have to search by distro_arch_series and combine the results.
-    # <https://bugs.launchpad.net/launchpad/+bug/904670>.
-    for distro_arch_series in series.architectures:
-        collections.append(
-            _lp_get_published_binaries(archive, distro_arch_series, **filters))
-    return chain(*collections)
+    lp_series = _lp_get_series(distribution, series)
+    return _iter_published_binaries_for_series(distribution, lp_series,
+            status=status, since=since)
 
 
 class PackageStatus(object):

=== modified file 'udd/tests/test_icommon.py'
--- udd/tests/test_icommon.py	2011-12-15 12:42:59 +0000
+++ udd/tests/test_icommon.py	2012-08-11 16:10:27 +0000
@@ -1,4 +1,5 @@
 from datetime import datetime, timedelta
+import itertools
 
 from bzrlib import tests
 
@@ -77,6 +78,8 @@
         super(TestIterPublishedBinaries, self).setUp()
         self.patch(icommon, '_lp_get_published_binaries',
                    self._fake_get_published_binaries)
+        self.patch(icommon, '_lp_get_series',
+                   self._fake_get_series)
 
     def _fake_get_published_binaries(self, archive, distro_arch_series,
                                      **filters):
@@ -87,6 +90,16 @@
         """
         return iter([archive, distro_arch_series, filters])
 
+    def _fake_get_series(self, distribution, name_or_version):
+        """A fake version of _lp_get_series.
+
+        Returns an object with an architectures attribute that will be
+        a list of architecture strings related to the series name.
+        """
+        archs = map(lambda x: name_or_version + 'arch' + x,
+                   map(str, range(3)))
+        return Struct(architectures=archs)
+
     def test_series_provided(self):
         # If a series is provided then iter_published_binaries will do one
         # call per distro_arch_series and then merge the results.
@@ -94,17 +107,15 @@
         since = object()
         archive = object()
         distribution = Struct(main_archive=archive)
-        series = Struct(architectures=['foo', 'bar', 'baz'])
+        series = 'series'
         binaries = icommon.iter_published_binaries(
             distribution, status=status, since=since, series=series)
         # We expect multiple calls to _lp_get_published_binaries. The iterable
         # results of these calls will be chained together.
-        self.assertEqual(
-            [archive, 'foo', {'status': status, 'created_since_date': since},
-             archive, 'bar', {'status': status, 'created_since_date': since},
-             archive, 'baz', {'status': status, 'created_since_date': since},
-             ],
-            list(binaries))
+        def make_results(arch):
+            return [archive, arch, {'status': status, 'created_since_date': since}]
+        results = map(make_results, self._fake_get_series(distribution, series).architectures)
+        self.assertEqual(list(itertools.chain(*results)), list(binaries))
 
     def test_series_not_provided(self):
         # If no series is provided, then iter_published_binaries will do one