Merge lp:~cjwatson/launchpad/export-change-override into lp:launchpad

[Colin Watson]

Export SourcePackagePublishingHistory.changeOverride and BinaryPackagePublishingHistory.changeOverride.

== Summary ==

change-override.py, used frequently by Ubuntu archive administrators, requires direct database access (bug 853831).

== Proposed fix ==

Export SPPH.changeOverride and BPPH.changeOverride. It will then be easy to write an API client replacing change-override.py.

== Implementation details ==

The exports require a slight rearrangement of the *PPH interfaces, creating IBinaryPackagePublishingHistoryEdit and ISourcePackagePublishingHistoryEdit which inherit from the existing IPublishingEdit base class. It isn't possible to add changeOverride to IPublishingEdit directly because it has different parameters in the source and binary cases.

I moved ArchiveOverriderError from lp.soyuz.scripts.changeoverride to lp.soyuz.model.publishing, to stop the latter relying on the former.

In order to make this testable within the existing publishing webservice tests, I had to fix a buglet whereby changeOverride complains about a component override requiring a new archive even if it wasn't actually asked to change the component. This doesn't normally make any difference, but the webservice tests work in a PPA and in that case it does.

I did seriously contemplate refactoring the doctests here as unit tests and avoiding sampledata and the like, which would have made it much more straightforward to do some tests using a PRIMARY archive instead, and in fact I did start in on that; but I eventually decided it would make the branch too unwieldy and unclear. I've kept my progress on that around and will probably submit it as a separate branch at some point.

I took care to export these new methods only on devel.

This may not be desperately performant when overriding large numbers of binary packages. I'm not too worried about this as an operational showstopper, and I don't believe that this branch will slow any of the current interfaces down, but I'd take suggestions for alternative ways to approach the new interfaces; something on Archive maybe? (See also https://code.launchpad.net/~cjwatson/launchpad/queue-api/+merge/108967.)

== LOC Rationale ==

+134, but there's at minimum 210 lines of scripts/ftpmaster-tools/change-override.py and lib/lp/soyuz/scripts/changeoverride.py that I will remove once this has been deployed and I've written an API client, not to mention probably the best part of the 503 lines of lib/lp/soyuz/scripts/tests/test_changeoverride.py.

== Tests ==

bin/test -vvct xx-source-package-publishing.txt -t xx-binary-package-publishing.txt

== Demo and Q/A ==

Using lp-shell on qastaging, use *PPH.changeOverride on some randomly selected source and binary publication records (perhaps based on http://people.canonical.com/~ubuntu-archive/component-mismatches.txt), and verify that this causes the corresponding +publishinghistory entries to change.

== Lint ==

Pre-existing lint, not worth fixing here:

./lib/lp/soyuz/stories/webservice/xx-binary-package-publishing.txt
     192: want exceeds 78 characters.
     210: want exceeds 78 characters.
     220: source exceeds 78 characters.
     225: source exceeds 78 characters.
     230: source exceeds 78 characters.
     235: source exceeds 78 characters.
     240: source exceeds 78 characters.

[Graham Binns (gmb)]

Hi Colin,

Overall, I like this branch, and I'm happy for it to land (I'll just
have Robert hold you to your commitment to trim the LoC elsewhere ;)).

Just one question:

188 + # Really IBinaryPackagePublishingHistory, patched in
189 + # _schema_circular_imports.py.
190 + @operation_returns_entry(Interface)
191 + @operation_parameters(
192 + new_component=TextLine(title=u"The new component name."),
193 + new_section=TextLine(title=u"The new section name."),
194 + new_priority=TextLine(title=u"The new priority name."))
195 + @export_write_operation()
196 + @operation_for_version("devel")
197 + def changeOverride(new_component=None, new_section=None,
198 + new_priority=None):

I'm confused as to why you're using TextLine here for new_priority
(unless there's some pitfall in using copy_field() between different
interfaces; I don't know about that). You should be able to use
copy_field() here to copy the relevant field ( in this case I'm guessing
it'd be IBinaryPublishingHistory.priority), which would mean that you
don't have to do a lookup for the priority in name_priority_map later on
(line 297 of the diff)- lazr.restful would take care of that for you.

You can see an example of this in lib/lp/bugs/interfaces/bugtask.py:750.

review: Needs Fixing (code)

[Colin Watson (cjwatson)]

I tried this suggestion (http://paste.ubuntu.com/1038961/), but two problems: firstly, the title= override doesn't seem to take so the title is wrong in apidoc (it says "The priority being published into", which is less helpful); secondly, tests fail with http://paste.ubuntu.com/1038963/ suggesting that the magic lookup in lazr.restful doesn't actually happen. Do you know what I might be doing wrong?

[Graham Binns (gmb)]

From IRC:
<gmb> cjwatson, Okay, it looks like copy_field isn't doing what it should. Revert to your earlier solution and add a comment explaining why you're not using copy_field. r=me on everything else.

review: Approve (code)