Merge lp://qastaging/~cjwatson/launchpad/copies-respect-new into lp://qastaging/launchpad

Proposed by Colin Watson
Status: Work in progress
Proposed branch: lp://qastaging/~cjwatson/launchpad/copies-respect-new
Merge into: lp://qastaging/launchpad
Diff against target: 146 lines (+67/-8)
3 files modified
lib/lp/soyuz/configure.zcml (+1/-2)
lib/lp/soyuz/model/packagecopyjob.py (+27/-6)
lib/lp/soyuz/tests/test_packagecopyjob.py (+39/-0)
To merge this branch: bzr merge lp://qastaging/~cjwatson/launchpad/copies-respect-new
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+221529@code.qastaging.launchpad.net

Commit message

Send copies to NEW if they contain binaries without overrides.

Description of the change

Make binaryful copies respect NEW. This is not ideal because it results in entries in the NEW queue which don't really explain why they're there; however, it's a first step towards a more reasonable representation of copies. This is particularly important to make sure that there's an opportunity for packages staged in PPAs to receive proper review when copied into the Ubuntu archive.

To post a comment you must log in.
Revision history for this message
Adam Conrad (adconrad) wrote :

So, until the queue redesign copies remain opaque, which means that can't be overridden until they're actually accepted. Will this change at least make sure ancestry does the expected thing (the code certainly seems to mention ancestry) and makes new binaries match the overrides of the source? For most cases, this should end up being the desired behaviour.

Revision history for this message
William Grant (wgrant) wrote :

This is going to block any binary copies to series (or distros!) with fewer arches, but otherwise seems fine.

review: Approve (code)
Revision history for this message
Colin Watson (cjwatson) wrote :

> So, until the queue redesign copies remain opaque, which means that can't be
> overridden until they're actually accepted.

Right. I know that this is only part of what we need; the patch includes a bug reference for the other major part of the problem. That's bigger, though, and I think that making sure things land in NEW at all is more immediately pressing. It's at least somewhat useful: for example, you can at least reject an upload if it's inappropriate.

William and I think that making binary overrides work on copies may not actually require the full redesign, although that would still be valuable for other reasons (e.g. making diffs work properly would be unutterably tedious with what we have now). We'd stuff the override data into the PU's JSON metadata much as we do now with source uploads, probably in the same format that we currently return from PU.getBinaryProperties, and hook up PU.getBinaryProperties and PU.overrideBinary to that; and we'd have to fish out those overrides in the PCJ, pass them down to the copier, and make the copier handle binary overrides, which is probably the largest chunk of work.

But I didn't want to do that in the same branch as this change, because it'll be a bit longer and I've given William more than enough gigantic branches to review lately ...

> Will this change at least make
> sure ancestry does the expected thing (the code certainly seems to mention
> ancestry) and makes new binaries match the overrides of the source? For most
> cases, this should end up being the desired behaviour.

All this change does is sometimes suspend the copy job and create a queue entry for it in a few more places. If the queue entry is accepted, then the copy job will be reprocessed from the start, applying overrides exactly as before.

When we implement the scheme above, we'll apply the archive's override policy when calculating the overrides that we stuff into the PU, which will result in reasonable defaults.

Revision history for this message
Colin Watson (cjwatson) wrote :

> So, until the queue redesign copies remain opaque, which means that can't be
> overridden until they're actually accepted.

Right. I know that this is only part of what we need; the patch includes a bug reference for the other major part of the problem. That's bigger, though, and I think that making sure things land in NEW at all is more immediately pressing. It's at least somewhat useful: for example, you can at least reject an upload if it's inappropriate.

William and I think that making binary overrides work on copies may not actually require the full redesign, although that would still be valuable for other reasons (e.g. making diffs work properly would be unutterably tedious with what we have now). We'd stuff the override data into the PU's JSON metadata much as we do now with source uploads, probably in the same format that we currently return from PU.getBinaryProperties, and hook up PU.getBinaryProperties and PU.overrideBinary to that; and we'd have to fish out those overrides in the PCJ, pass them down to the copier, and make the copier handle binary overrides, which is probably the largest chunk of work.

But I didn't want to do that in the same branch as this change, because it'll be a bit longer and I've given William more than enough gigantic branches to review lately ...

> Will this change at least make
> sure ancestry does the expected thing (the code certainly seems to mention
> ancestry) and makes new binaries match the overrides of the source? For most
> cases, this should end up being the desired behaviour.

All this change does is sometimes suspend the copy job and create a queue entry for it in a few more places. If the queue entry is accepted, then the copy job will be reprocessed from the start, applying overrides exactly as before.

When we implement the scheme above, we'll apply the archive's override policy when calculating the overrides that we stuff into the PU, which will result in reasonable defaults.

Revision history for this message
Colin Watson (cjwatson) wrote :

(Sorry for the duplicate. I blame the hotel's captive portal for confusing me.)

Unmerged revisions

17033. By Colin Watson

Send copies to NEW if they contain binaries without overrides.

17032. By Colin Watson

Remove reference to non-existent setNew attribute.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.