Merge lp:~jml/udd/symbol-import into lp:udd

[Jonathan Lange]

I'm trying to do something across all Ubuntu packages. To do this, I need mostly what udd does, but I need to exclude Debian packages and sometimes I need to retry all failed jobs (particularly when debugging). This branch adds these options.

add_import_jobs.py
 * Call create_import_jobs, passing on the new include_debian config option
list_packages.py
 * Respect the new include_debian config option
mass_import.py
 * Use the new re-add all failed jobs method if the appropriate configuration option is set
pkgimport.conf:
 * Add new configuration options, with documentation
udd/icommon.py:
 * Import datetime and timedelta directly
 * Use timedelta(days=N) rather than manually multiplying
 * Refactor create_import_jobs to re-use some other code
 * Add a wrapper for the new config option for include_debian
 * Make create_import_jobs take an option on whether to include debian packages
 * Add a new method to the controller that will re-add all failed jobs
udd/lpapi.py:
 * General pyflakes cleanups

[James Westby (james-w)]

Hi,

Thanks for the branch, there are some nice cleanups in here to sweeten
the deal.

56 + if include_debian:
57 + for s in debian.series:
58 + collection = lpapi.lp_call(lpapi.call_with_limited_size,
59 + d_archive.getPublishedSources,
60 + status="Pending",
61 + distro_series=s, created_since_date=last_update)
62 + for publication in lpapi.iterate_collection(collection):
63 + packages = {}
64 + if publication.source_package_name in packages:
65 + continue
66 + packages[publication.source_package_name] = 0
67 + db.update_packages(packages)

Would this be better as a list of distribution names, rather than a boolean?

Likely someone is going to want to use this code with derived distros
at some point, and that change would seem to be more inline with that.

I'm not saying that you have to support derived distros in every way though :-)

I'm interested in why you changed import in one file from "from datetime import datetime"
to "import datetime" and changed another file in the opposite direction?

I'm sure this comment is going to annoy you, but...

367 - request = operation.wadl_method.request

I think that line should be there, as that code is taken from lazr.restfulclient,
and someone in the future looking to see if we can drop the code will have
to wonder if removing that dead code was significant.

It even annoys me :-)

Thanks,

James

review: Approve

[Jonathan Lange (jml)]

On Wed, Oct 5, 2011 at 4:57 PM, James Westby <email address hidden> wrote:
> Review: Approve
>
> Hi,
>
> Thanks for the branch, there are some nice cleanups in here to sweeten
> the deal.
>
> 56      + if include_debian:
> 57      + for s in debian.series:
> 58      + collection = lpapi.lp_call(lpapi.call_with_limited_size,
> 59      + d_archive.getPublishedSources,
> 60      + status="Pending",
> 61      + distro_series=s, created_since_date=last_update)
> 62      + for publication in lpapi.iterate_collection(collection):
> 63      + packages = {}
> 64      + if publication.source_package_name in packages:
> 65      + continue
> 66      + packages[publication.source_package_name] = 0
> 67      + db.update_packages(packages)
>
> Would this be better as a list of distribution names, rather than a boolean?
>

Yeah, that's how I started off, and then I saw just how much 'debian'
and 'ubuntu' were spread throughout the code. (e.g., the manually
maintained distro → series maps).

> Likely someone is going to want to use this code with derived distros
> at some point, and that change would seem to be more inline with that.
>
> I'm not saying that you have to support derived distros in every way though :-)
>

OK. I'll do what I can and see what turns up.

>
> I'm interested in why you changed import in one file from "from datetime import datetime"
> to "import datetime" and changed another file in the opposite direction?
>

Umm, I didn't?

>
> I'm sure this comment is going to annoy you, but...
>
> 367     - request = operation.wadl_method.request
>
> I think that line should be there, as that code is taken from lazr.restfulclient,
> and someone in the future looking to see if we can drop the code will have
> to wonder if removing that dead code was significant.
>
> It even annoys me :-)
>

OK. I didn't see a comment there saying it was taken from
lazr.restfulclient. I'll add a comment along those lines when I
restore the removed line.

Thanks!
jml

[James Westby (james-w)]

On Wed, 05 Oct 2011 16:07:25 -0000, Jonathan Lange <email address hidden> wrote:
> Yeah, that's how I started off, and then I saw just how much 'debian'
> and 'ubuntu' were spread throughout the code. (e.g., the manually
> maintained distro → series maps).

Ah, that's true. They are a pain, but I don't know of a way to get rid
of them.

Perhaps they aren't needed for anything no in ('debian', 'ubuntu')
though.

> > Likely someone is going to want to use this code with derived distros
> > at some point, and that change would seem to be more inline with that.
> >
> > I'm not saying that you have to support derived distros in every way though :-)
> >
>
> OK. I'll do what I can and see what turns up.

We can always deprecate the debian option later, so if it's too much
work we'll just do that.

> >
> > I'm interested in why you changed import in one file from "from datetime import datetime"
> > to "import datetime" and changed another file in the opposite direction?
> >
>
> Umm, I didn't?

Sorry, I have no idea where I saw that, my mistake entirely.

> > I'm sure this comment is going to annoy you, but...
> >
> > 367     - request = operation.wadl_method.request
> >
> > I think that line should be there, as that code is taken from lazr.restfulclient,
> > and someone in the future looking to see if we can drop the code will have
> > to wonder if removing that dead code was significant.
> >
> > It even annoys me :-)
> >
>
> OK. I didn't see a comment there saying it was taken from
> lazr.restfulclient. I'll add a comment along those lines when I
> restore the removed line.

Good idea, thanks.

James

[Vincent Ladeuil (vila)]

Thanks for working on this (added comments *especially* appreciated).

> sometimes I need to retry all failed jobs (particularly when debugging)

and

239 +
240 + def add_failed_jobs(self):
241 + """Add all failed jobs to the queue."""
242 + c = self.conn.cursor()
243 + try:
244 + rows = c.execute('select * from %s' % self.FAILURES_TABLE).fetchall()
245 + for row in rows:
246 + c.execute(self.FAILURES_TABLE_DELETE, (row[0],))
247 + self._add_job(c, row[0], self.JOB_TYPE_PRIORITY)
248 + self.conn.commit()
249 + except:
250 + self.conn.rollback()
251 + raise
252 + finally:
253 + c.close()
254 +

makes me think that you want to add an option to requeue_package.py instead.
This script can be run without stopping mass_import so if we can share the
code bewtween them it will be even nicer (i.e. I think adding the capability
is a good thing even if the package importer won't use it for now).

You're duplicating part of retry/_retry here without fully implementing what
they do though (recording the failures in OLD_FAILURES_TABLE is helpful in
the long term).

196 + for spph in iter_published_sources(lp, lp.distributions['ubuntu'],

Damn, I'm quite good at deciphering aconyms but spph resists me... sp ==
status pending ?

Can I have more hints ?

;)

[Vincent Ladeuil (vila)]

Oh ! I had this page opened since yesterday evening and didn't see james review nor your reply.

[James Westby (james-w)]

On Thu, 06 Oct 2011 11:36:46 -0000, Vincent Ladeuil <email address hidden> wrote:
> 196 + for spph in iter_published_sources(lp, lp.distributions['ubuntu'],
>
> Damn, I'm quite good at deciphering aconyms but spph resists me... sp ==
> status pending ?
>
> Can I have more hints ?

"SourcePackagePublishingHistory" a Launchpad term for a record of
publishing a source package in an archive.

Thanks,

James

[Jonathan Lange (jml)]

On Thu, Oct 6, 2011 at 12:36 PM, Vincent Ladeuil <email address hidden> wrote:
> Thanks for working on this (added comments *especially* appreciated).
>
>> sometimes I need to retry all failed jobs (particularly when debugging)
>
> and
>
> 239     +
> 240     + def add_failed_jobs(self):
> 241     + """Add all failed jobs to the queue."""
> 242     + c = self.conn.cursor()
> 243     + try:
> 244     + rows = c.execute('select * from %s' % self.FAILURES_TABLE).fetchall()
> 245     + for row in rows:
> 246     + c.execute(self.FAILURES_TABLE_DELETE, (row[0],))
> 247     + self._add_job(c, row[0], self.JOB_TYPE_PRIORITY)
> 248     + self.conn.commit()
> 249     + except:
> 250     + self.conn.rollback()
> 251     + raise
> 252     + finally:
> 253     + c.close()
> 254     +
>
> makes me think that you want to add an option to requeue_package.py instead.
> This script can be run without stopping mass_import so if we can share the
> code bewtween them it will be even nicer (i.e. I think adding the capability
> is a good thing even if the package importer won't use it for now).
>
> You're duplicating part of retry/_retry here without fully implementing what
> they do though (recording the failures in OLD_FAILURES_TABLE is helpful in
> the long term).
>

To be honest, I don't see it. retry() is a big function of unclear
purpose. What seems to be clear is that it's meant to work on one
package at a time. Same with requeue_packages.py.

add_failed_jobs is duplicating add_jobs_for_interrupted. If my new one
is missing functionality, then so is the existing one.

jml

[lp:~jml/udd/symbol-import]

524. By Jonathan Lange on 2011-10-06

Restore the duplication.

525. By Jonathan Lange on 2011-10-06

Change include_debian to distribution list.

[Jonathan Lange (jml)]

r525 makes the include_debian to distribution list change. Worth reviewing before landing.

[James Westby (james-w)]

38 # XXX: This used to do "Published" for Ubuntu and "Pending" for
39 # Debian (which was really Ubuntu). Now we do both.

Hi,

I think we can just do Published now:

  https://bugs.launchpad.net/udd/+bug/865483

but if you don't want to make the two changes at once then that's ok.

49 # XXX: Don't know why there's a special check here for
50 # main packages in the current series. Used to be for
51 # Ubuntu only, now we check for all distros.

That's so that we record "important" packages in the db, so
we can start with those. Changing to all distros is fine.

That change looks good though. I'll let vila respond on the
comments that he made.

Thanks,

James

[Vincent Ladeuil (vila)]

> add_failed_jobs is duplicating add_jobs_for_interrupted. If my new one
> is missing functionality, then so is the existing one.

No, retry indeed works for a single package, but it takes care about keeping track of previous failures which is the part that is missing. add_jobs_for_interrupted is catching up for... interrupted jobs which did not fail but needs to be spawn again.

[Jonathan Lange (jml)]

On Thu, Oct 6, 2011 at 3:51 PM, Vincent Ladeuil <email address hidden> wrote:
>> add_failed_jobs is duplicating add_jobs_for_interrupted. If my new one
>> is missing functionality, then so is the existing one.
>
> No, retry indeed works for a single package, but it takes care about keeping track of previous failures which is the part that is missing. add_jobs_for_interrupted is catching up for... interrupted jobs which did not fail but needs to be spawn again.

They are recorded as failures using a sentinel. I fail to see why they
are different, or, to be honest, what would be required to make retry
usable for my needs.

jml

[Vincent Ladeuil (vila)]

> On Thu, Oct 6, 2011 at 3:51 PM, Vincent Ladeuil <email address hidden> wrote:
> >> add_failed_jobs is duplicating add_jobs_for_interrupted. If my new one
> >> is missing functionality, then so is the existing one.
> >
> > No, retry indeed works for a single package, but it takes care about keeping
> track of previous failures which is the part that is missing.
> add_jobs_for_interrupted is catching up for... interrupted jobs which did not
> fail but needs to be spawn again.
>
> They are recorded as failures using a sentinel. I fail to see why they
> are different, or, to be honest, what would be required to make retry
> usable for my needs.

sentinel is a hack not a true failure tracked like the others

I won't try to argue and delay the landing of this feature, I think you haven't use requeue_package and may not need it in your actual workflow (if your imports never fail spuriously or permanently for example).

The package importer will never retry all failures blindly either.

As long as the uses case don't overlap I'm fine with the small code duplication (especially since the actual design makes it hard to resolve (one package/several packages, --all-of-type magic, etc).

Keep in mind, though, that the mass_import script relies on transient failures created with requeue_package --auto to avoid failure spikes during lp downtimes.

[Jonathan Lange (jml)]

On Thu, Oct 6, 2011 at 6:03 PM, Vincent Ladeuil <email address hidden> wrote:
>> On Thu, Oct 6, 2011 at 3:51 PM, Vincent Ladeuil <email address hidden> wrote:
>> >> add_failed_jobs is duplicating add_jobs_for_interrupted. If my new one
>> >> is missing functionality, then so is the existing one.
>> >
>> > No, retry indeed works for a single package, but it takes care about keeping
>> track of previous failures which is the part that is missing.
>> add_jobs_for_interrupted is catching up for... interrupted jobs which did not
>> fail but needs to be spawn again.
>>
>> They are recorded as failures using a sentinel. I fail to see why they
>> are different, or, to be honest, what would be required to make retry
>> usable for my needs.
>
> sentinel is a hack not a true failure tracked like the others
>
> I won't try to argue and delay the landing of this feature, I think you haven't use requeue_package and may not need it in your actual workflow (if your imports never fail spuriously or permanently for example).
>
> The package importer will never retry all failures blindly either.
>
> As long as the uses case don't overlap I'm fine with the small code duplication (especially since the actual design makes it hard to resolve (one package/several packages, --all-of-type magic, etc).
>
> Keep in mind, though, that the mass_import script relies on transient failures created with requeue_package  --auto to avoid failure spikes during lp downtimes.

OK, thanks. I think I need more confidence in this codebase before
attempting a change like that.

Would one of you please merge my branch for me?

Thanks,
jml

[lp:~jml/udd/symbol-import]

526. By Jonathan Lange on 2011-10-06

Don't bother with Pending status.
Explain why we are treating main/current specially (sort of)

=== modified file 'add_import_jobs.py'
--- add_import_jobs.py	2011-07-16 09:41:49 +0000
+++ add_import_jobs.py	2011-10-06 19:09:54 +0000
@@ -6,7 +6,6 @@
 from udd import lpapi
 from udd import paths
 
-
 lock = icommon.lock_add_import_jobs()
 if lock is None:
     print "Another instance of add_import_jobs is already running."
@@ -14,7 +13,6 @@
 try:
     lp = lpapi.get_lp()
     status_db = icommon.StatusDatabase(paths.sqlite_file)
-
     icommon.create_import_jobs(lp, status_db)
 finally:
     lock.close()

=== modified file 'list_packages.py'
--- list_packages.py	2011-07-19 09:36:38 +0000
+++ list_packages.py	2011-10-06 19:09:54 +0000
@@ -8,6 +8,7 @@
 from launchpadlib.errors import HTTPError
 
 from udd import icommon
+from udd import iconfig
 from udd import lpapi
 from udd import paths
 
@@ -18,46 +19,38 @@
     sys.exit(0)
 try:
     lp = lpapi.get_lp()
-    debian = lp.distributions['ubuntu']
-    ubuntu = lp.distributions['ubuntu']
-    d_archive = debian.main_archive
-    u_archive = ubuntu.main_archive
-    current_u_series_name = ubuntu.current_series.name
-
 
     db = icommon.PackageDatabase(paths.sqlite_package_file)
 
     last_update = db.last_update()
     if last_update is not None:
-        last_update = last_update - datetime.timedelta(0, 86400)
+        last_update = last_update - datetime.timedelta(days=1)
         last_update = last_update.isoformat()
 
-    for s in ubuntu.series:
-        collection = lpapi.lp_call(lpapi.call_with_limited_size,
-                u_archive.getPublishedSources,
-                status="Published",
-                distro_series=s, created_since_date=last_update)
-        for publication in lpapi.iterate_collection(collection):
-            packages = {}
-            if publication.source_package_name in packages:
-                continue
-            if (s.name == current_u_series_name and
-                publication.component_name == "main"):
-                packages[publication.source_package_name] = 1
-            else:
-                packages[publication.source_package_name] = 0
-            db.update_packages(packages)
-    for s in debian.series:
-        collection = lpapi.lp_call(lpapi.call_with_limited_size,
-                d_archive.getPublishedSources,
-                status="Pending",
-                distro_series=s, created_since_date=last_update)
-        for publication in lpapi.iterate_collection(collection):
-            packages = {}
-            if publication.source_package_name in packages:
-                continue
-            packages[publication.source_package_name] = 0
-            db.update_packages(packages)
+    config = iconfig.Iconfig()
+    distributions = config.get_list('pkgimport.distributions')
+
+    for distro_name in distributions:
+        distro = lp.distributions[distro_name]
+        archive = distro.main_archive
+        current_series_name = distro.current_series.name
+        for s in distro.series:
+            collection = lpapi.lp_call(lpapi.call_with_limited_size,
+                    archive.getPublishedSources,
+                    status='Published',
+                    distro_series=s, created_since_date=last_update)
+            for publication in lpapi.iterate_collection(collection):
+                packages = {}
+                if publication.source_package_name in packages:
+                    continue
+                # Packages in the current series in the 'main' component are
+                # considered to be more important than the rest.
+                if (s.name == current_series_name and
+                    publication.component_name == "main"):
+                    packages[publication.source_package_name] = 1
+                else:
+                    packages[publication.source_package_name] = 0
+                db.update_packages(packages)
 
 except HTTPError, e:
     sys.stderr.write(e.content + "\n")

=== modified file 'mass_import.py'
--- mass_import.py	2011-09-29 11:41:05 +0000
+++ mass_import.py	2011-10-06 19:09:54 +0000
@@ -388,14 +388,18 @@
         # Start with an 8-way Driver
         self.driver = ImportDriver(8)
 
-    def run(self):
+    def run(self, retry_all=False):
         lock = icommon.lock_main()
         if lock is None:
             logger.info("Another main process is running")
             raise Stop
         try:
-            # First, re-queue previously interrupted jobs.
-            self.status_db.add_jobs_for_interrupted()
+            if retry_all:
+                # We have been configured to retry _all_ failed jobs.
+                self.status_db.add_failed_jobs()
+            else:
+                # Re-queue previously interrupted jobs.
+                self.status_db.add_jobs_for_interrupted()
             self.driver.start()
             self.driver.started.wait()
             while True:
@@ -470,9 +474,11 @@
 signal.signal(signal.SIGTERM, die)
 
 
+config = iconfig.Iconfig()
+retry_all = config.get('pkgimport.retry_all_failed_jobs') == 'true'
 # Run the controller
 try:
-    controller.run()
+    controller.run(retry_all)
 except HTTPError, e:
     logger.critical(e.content)
     raise

=== modified file 'pkgimport.conf'
--- pkgimport.conf	2011-09-27 11:41:40 +0000
+++ pkgimport.conf	2011-10-06 19:09:54 +0000
@@ -20,3 +20,12 @@
 # Script that imports the package. Must be executable and take a single
 # argument: the name of the package to import.
 pkgimport.import_script = /srv/package-import.canonical.com/new/scripts/import_package.py
+
+# Which distributions to operate on.  Normally "ubuntu, debian".  No default.
+# Affects behaviour of add_import_jobs.py and list_packages.py.
+pkgimport.distributions = ubuntu,debian
+
+# Whether or not to retry all failed jobs. 'true' means to retry all failed
+# jobs. Anything else means retry only jobs that were currently running when
+# the supervisor died. Defaults to 'false' if unset.
+pkgimport.retry_all_failed_jobs = false

=== modified file 'udd/icommon.py'
--- udd/icommon.py	2011-09-19 15:48:31 +0000
+++ udd/icommon.py	2011-10-06 19:09:54 +0000
@@ -1,4 +1,4 @@
-import datetime
+from datetime import datetime, timedelta
 import errno
 import fcntl
 import operator
@@ -308,7 +308,7 @@
     checked = set()
     newest_published = last_known_published
     for publication in publications:
-        published_time = datetime.datetime.strptime(
+        published_time = datetime.strptime(
             str(publication.date_created)[:19], "%Y-%m-%d %H:%M:%S")
         name = publication.source_package_name
         checked.add(name)
@@ -317,34 +317,49 @@
     status_db.add_import_jobs(checked, newest_published)
 
 
+def get_last_import_time(status_db, earliest_date=None):
+    """Get the last import time from status db.
+
+    Adjusts for safety, so that we don't miss any packages on the new
+    synchronization. If ``earliest_date`` provided, then never return a date
+    earlier than that, even if we've never synced before.
+    """
+    last_known_published = status_db.last_import_time()
+    if last_known_published:
+        # Safety buffer so we don't accidentally miss other syncs.
+        return last_known_published - timedelta(minutes=10)
+    return earliest_date
+
+
 def create_import_jobs(lp, status_db):
-    last_known_published = status_db.last_import_time()
-    last_published_call = None
-    if last_known_published is not None:
-        last_known_published = (last_known_published
-                                - datetime.timedelta(0, 600))
-    else:
-        # When starting from scratch, limit the query to the last ten days or
-        # lp timeout (https://bugs.launchpad.net/udd/+bug/717109)
-        last_known_published = (datetime.datetime.now()
-                                - datetime.timedelta(0, 10 * 24 * 3600))
-    last_published_call = last_known_published.isoformat()
+    # When starting from scratch, limit the query to the last ten days or
+    # lp timeout (https://bugs.launchpad.net/udd/+bug/717109)
+    last_known_published = get_last_import_time(
+        status_db, earliest_date=datetime.now() - timedelta(days=10))
     # We want all the new Published records since the last run
     publications = set()
+    config = iconfig.Iconfig()
+    distributions = config.get_list('pkgimport.distributions')
+    for distro in distributions:
+        for spph in iter_published_sources(lp, lp.distributions[distro],
+                                           status='Published',
+                                           since=last_known_published.isoformat()):
+            publications.add(spph)
+    import_from_publications(status_db, publications, last_known_published)
+
+
+def iter_published_sources(lp, distribution, status='Published', since=None,
+                           series=None):
+    filters = {'status': status}
+    if since:
+        filters['created_since_date'] = since
+    if series:
+        series = distribution.getSeries(name_or_version=series)
+        filters['distro_series'] = series
+    archive = distribution.main_archive
     collection = lpapi.lp_call(
-        lpapi.call_with_limited_size,
-        lp.distributions['ubuntu'].main_archive.getPublishedSources,
-        status="Published",
-        created_since_date=last_published_call)
-    for p in lpapi.iterate_collection(collection):
-        publications.add(p)
-    collection = lpapi.lp_call(lpapi.call_with_limited_size,
-            lp.distributions['debian'].main_archive.getPublishedSources,
-            status="Pending",
-            created_since_date=last_published_call)
-    for p in lpapi.iterate_collection(collection):
-        publications.add(p)
-    import_from_publications(status_db, publications, last_known_published)
+        lpapi.call_with_limited_size, archive.getPublishedSources, **filters)
+    return lpapi.iterate_collection(collection)
 
 
 class PackageStatus(object):
@@ -450,6 +465,22 @@
             c.execute('insert into %s values (?, ?, ?, 0)'
                     % self.FAILURES_TABLE, (package, reason, now))
 
+
+    def add_failed_jobs(self):
+        """Add all failed jobs to the queue."""
+        c = self.conn.cursor()
+        try:
+            rows = c.execute('select * from %s' % self.FAILURES_TABLE).fetchall()
+            for row in rows:
+                c.execute(self.FAILURES_TABLE_DELETE, (row[0],))
+                self._add_job(c, row[0], self.JOB_TYPE_PRIORITY)
+            self.conn.commit()
+        except:
+            self.conn.rollback()
+            raise
+        finally:
+            c.close()
+
     def add_jobs_for_interrupted(self):
         c = self.conn.cursor()
         try:
@@ -470,7 +501,7 @@
         # Search the active jobs for the given package ordered by job type
         # (priority, new, retry)
         rows = c.execute(self.JOBS_TABLE_FIND, (package, 1)).fetchall()
-        now = datetime.datetime.utcnow()
+        now = datetime.utcnow()
         first = now
         job_id = 0
         for row in rows:
@@ -505,7 +536,7 @@
     def finish_job(self, package, job_id, success, output):
         c = self.conn.cursor()
         try:
-            now = datetime.datetime.utcnow()
+            now = datetime.utcnow()
             if job_id > 0:
                 row = c.execute(self.JOBS_TABLE_SELECT_JOB,
                                 (job_id,)).fetchone()
@@ -530,7 +561,7 @@
 
     def _add_job(self, c, package, job_type):
         c.execute(self.JOBS_TABLE_INSERT, (None, package, 1, job_type,
-                    datetime.datetime.utcnow(), None, None))
+                    datetime.utcnow(), None, None))
 
     def add_jobs(self, packages, job_type):
         c = self.conn.cursor()
@@ -739,8 +770,8 @@
             return False
         info.auto_retry = True
         info.auto_retry_time = (
-            info.timestamp + datetime.timedelta(0, self.AUTO_RETRY_SECONDS))
-        if info.auto_retry_time > datetime.datetime.utcnow():
+            info.timestamp + timedelta(seconds=self.AUTO_RETRY_SECONDS))
+        if info.auto_retry_time > datetime.utcnow():
             # Too early to retry
             return False
         row = c.execute(self.OLD_FAILURES_TABLE_FIND, (info.name,)).fetchone()
@@ -930,7 +961,7 @@
                 else:
                     c.execute(self.PACKAGE_TABLE_INSERT, (package, main))
             c.execute('insert into %s values (?)' % self.UPDATE_TABLE,
-                    (datetime.datetime.utcnow(),))
+                    (datetime.utcnow(),))
             self.conn.commit()
         except:
             self.conn.rollback()
@@ -1206,9 +1237,8 @@
     def _set_counts(self, table, queued, failed):
         c = self.conn.cursor()
         try:
-            rows = c.execute("insert into %s values (?, ?, ?)"
-                    % table,
-                    (datetime.datetime.utcnow(), queued, failed))
+            c.execute("insert into %s values (?, ?, ?)" % table,
+                      (datetime.utcnow(), queued, failed))
             self.conn.commit()
         except:
             self.conn.rollback()
@@ -1345,7 +1375,7 @@
 
     def side_branch_location(self, distro, release, suite):
         distro, suite = self._translate_suite(distro, suite)
-        now = datetime.datetime.utcnow()
+        now = datetime.utcnow()
         now_str = now.strftime("%Y%m%d%H%M")
         return ("bzr+ssh://%s/~%s/%s/%s/%s/%s-%s" % (
             self._get_host_name(),
@@ -1497,7 +1527,7 @@
 
     def side_branch_location(self, distro, release, suite):
         distro, suite = self._translate_suite(distro, suite)
-        now = datetime.datetime.utcnow()
+        now = datetime.utcnow()
         now_str = now.strftime("%Y%m%d%H%M")
         return os.path.join(self.branches_dir, "%s-%s-%s"
                             % (distro, suite, now_str))
@@ -1544,7 +1574,7 @@
 
     def side_branch_location(self, distro, release, suite):
         distro, suite = self._translate_suite(distro, suite)
-        now = datetime.datetime.utcnow()
+        now = datetime.utcnow()
         now_str = now.strftime("%Y%m%d%H%M")
         return os.path.join(paths.updates_dir, self.package,
                 suite + "-%s" % now_str)

=== modified file 'udd/iconfig.py'
--- udd/iconfig.py	2011-06-15 00:13:56 +0000
+++ udd/iconfig.py	2011-10-06 19:09:54 +0000
@@ -62,6 +62,12 @@
                 break
         return value
 
+    def get_list(self, name):
+        items = self.get(name)
+        if not items:
+            return []
+        return [item.strip() for item in items.split(',')]
+
 
 class Iconfig(ConfigStack):
     """An import packager configuration taking ``locations.conf`` into account.

=== modified file 'udd/lpapi.py'
--- udd/lpapi.py	2011-08-11 06:28:31 +0000
+++ udd/lpapi.py	2011-10-06 19:09:54 +0000
@@ -1,5 +1,4 @@
 import cgi
-import operator
 import os
 import random
 import simplejson
@@ -13,7 +12,7 @@
 
 from launchpadlib.credentials import Credentials
 from launchpadlib.errors import HTTPError
-from launchpadlib.launchpad import Launchpad, EDGE_SERVICE_ROOT
+from launchpadlib.launchpad import Launchpad
 
 from udd import paths
 
@@ -160,6 +159,9 @@
 
 def call_with_limited_size(operation, size=20, *args, **kwargs):
     """Invoke the method and process the result."""
+    # XXX: Copied from lazr.restfulclient.resource.NamedOperation.__call__ and
+    # changed to be a function that takes a NamedOperation, rather than a
+    # method on it, also to set the ws.size in the call.
     if len(args) > 0:
         raise TypeError('Method must be called with keyword args.')
     http_method = operation.wadl_method.name