Code review comment for lp:~michael.nelson/launchpad/510331-syncsources-latest-pub

Revision history for this message
Michael Nelson (michael.nelson) wrote :

= Summary =
Fixes 510331 by ensuring that syncSources only finds pub records that are currently PUBLISHED.

== Proposed fix ==

== Pre-implementation notes ==

See note on the bug, as well as:
<noodles> ok, sounds good. Also with bug 510331
<bigjools> noodles: yeah that one's easy
<noodles> IArchive.syncSource() already allows you to specify the version... but with the solution you've suggested for syncSources()
<bigjools> ?
<noodles> it would mean that we'd not copy deleted sources even when there is no published one... is that expected?
<bigjools> yes, I think so
<noodles> OK. easy then.
<bigjools> well, would you expect it to?
<noodles> I'm imagining using the web ui, looking at a bunch of deleted sources and selecting them...
<bigjools> that's syncSource behaviour really
<bigjools> syncSources is more automatic
<bigjools> but we can make it explicit in the docstring
<noodles> OK.

== Implementation details ==

I wasn't sure of the best way to test the private helper method. I ended up going with removeSecurityProxy() for the test, but would be happy to hear about a neater solution?

== Tests ==
bin/test -vvt doc/archive.txt -t TestCollectLatestPublishedSources

== Demo and Q/A ==

We can Q/A this using the API on dogfood (or edge).

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/soyuz/doc/archive.txt
  lib/lp/soyuz/tests/test_archive.py
  lib/lp/soyuz/model/archive.py

== Pylint notices ==

lib/lp/soyuz/model/archive.py
    1099: [C0301] Line too long (81/78) -- fixed.
    14: [F0401] Unable to import 'lazr.lifecycle.event' (No module named lifecycle)

« Back to merge proposal