Merge lp:~cjwatson/launchpad/copy-ddtp into lp:launchpad
Status: | Merged |
---|---|
Approved by: | Colin Watson |
Approved revision: | no longer in the source branch. |
Merged at revision: | 15598 |
Proposed branch: | lp:~cjwatson/launchpad/copy-ddtp |
Merge into: | lp:launchpad |
Diff against target: |
225 lines (+85/-19) 7 files modified
lib/lp/archivepublisher/ddtp_tarball.py (+12/-2) lib/lp/archivepublisher/debian_installer.py (+5/-2) lib/lp/archivepublisher/dist_upgrader.py (+5/-2) lib/lp/archivepublisher/tests/test_ddtp_tarball.py (+32/-1) lib/lp/archivepublisher/uefi.py (+5/-2) lib/lp/soyuz/scripts/custom_uploads_copier.py (+2/-3) lib/lp/soyuz/scripts/tests/test_custom_uploads_copier.py (+24/-7) |
To merge this branch: | bzr merge lp:~cjwatson/launchpad/copy-ddtp |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Benji York (community) | code | Approve | |
Review via email: mp+111688@code.launchpad.net |
Commit message
Teach CustomUploadsCopier to copy ddtp-tarball files.
Description of the change
== Summary ==
Bug 827941 reports that there's another custom upload type, namely ddtp-tarball, that needs to be accounted for in CustomUploadsCopier so that it gets copied to new distroseries. (Bug 672314 has an example of a concrete problem that I think has this as its root cause.)
== Proposed fix ==
At one point this would have been fiddly, but following https:/
== LOC Rationale ==
+46. However, this is part of an arc as follows:
https:/
https:/
this branch (+46)
a probable future branch to fix P-a-s handling in direct copies (should be <100)
removal of unembargo-package, delayed copies, ubuntu-security celebrity, etc. (at least -600)
Even fairly conservatively, this arc should end up well in the black.
== Tests ==
bin/test -vvct test_ddtp_tarball -t test_custom_
== Demo and Q/A ==
It would be best to wait until after https:/
I won't be able to land and QA this until the issue I spotted in https:/
In ddtp_tarball.py ( line 23 of the diff) there is this line:
_, component, _ = tarfile_ path.split( "_")
Since you are interested in underscores in the file name and not basename( tarfile_ path).split( "_") would be
the entire path, os.path.
better.
It would also be good to have some tests with underscores in the path
that show that the method handles them correctly.
Using the ValueError as an way of determining that the destructuring
assignment failed seems a bit indirect as far as input validation goes.
Why is getSeriesKey a classmethod when "cls" isn't used? Maybe a
staticmethod is more appropriate.
How about this for getSeriesKey?
@staticmethod tarfile_ path): basename( tarfile_ path)
def getSeriesKey(
fn = os.path.
if fn.count('_') != 2:
# The file name does not match the pattern, so no series key
# can be extracted.
return None
return fn.split('_')[1]