Merge lp:~3v1n0/ubuntu-archive-tools/sru-review-bileto-support into lp:ubuntu-archive-tools

Proposed by Marco Trevisan (Treviño)
Status: Merged
Approved by: Łukasz Zemczak
Approved revision: 1217
Merged at revision: 1485
Proposed branch: lp:~3v1n0/ubuntu-archive-tools/sru-review-bileto-support
Merge into: lp:ubuntu-archive-tools
Diff against target: 104 lines (+40/-18)
1 file modified
sru-review (+40/-18)
To merge this branch: bzr merge lp:~3v1n0/ubuntu-archive-tools/sru-review-bileto-support
Reviewer Review Type Date Requested Status
Łukasz Zemczak Approve
Brian Murray (community) Approve
Andy Whitcroft Approve
Review via email: mp+364193@code.launchpad.net

Description of the change

The long awaited feature from the SRU team having to deal with bileto sync's is there... :)

To post a comment you must log in.
1210. By Marco Trevisan (Treviño)

sur-review: Add support to show diffs coming from bileto PPAs

When handling a source upload that is a sync and is coming from a ppa owned by
~ci-train-ppa-service, we can get the debdiff directly from the ppa description, as
bileto generates it.

So, parse such text looking for the proper uri and support both plain-text and gzipped
diff files when showing them.

Revision history for this message
Andy Whitcroft (apw) wrote :

Overall this look ok to me. Some nits in-line.

review: Approve
1211. By Marco Trevisan (Treviño)

sru-review: Make Diff easier to read

1212. By Marco Trevisan (Treviño)

sru-review: Figure out if we have to gunzip a diff from its URI

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Sorry for the delay, I forgot about the review :(

Anyways, I've addressed the required changes I hope it's fine.

1213. By Marco Trevisan (Treviño)

sru-review: Figure out if we have to gunzip a diff from its URI

1214. By Marco Trevisan (Treviño)

Merge with trunk

1215. By Marco Trevisan (Treviño)

sru-review: Properly handle diff uris for re-created PPAs

In these cases the PPA is named something like bileto-NNN.R where R is the
rebuild number (normally 1).

The bileto name however, keeps being bileto-NNNN and so the regex should be
adapted accordingly

Revision history for this message
Brian Murray (brian-murray) wrote :

This is great thanks for working on this and updating it. One thing I noticed today when testing it was that bileto's "diff" includes the output of diffstat (e.g. https://objectstorage.prodstack4-5.canonical.com/v1/AUTH_39a8dbb93caf4ec889f8a1b7f69885db/bileto-4610/2021-07-09_10:06:30/focal_xtables-addons_content.diff) and then sru-review can add additional diffstat. This seemed rather redundant to me so I played with the code a bit and made the following change:

--- old/sru-review 2020-06-05 13:56:50 +0000
+++ new/sru-review 2021-07-13 23:18:49 +0000
@@ -43,6 +43,7 @@
 import webbrowser

 from contextlib import ExitStack
+from itertools import dropwhile
 from launchpadlib.launchpad import Launchpad
 from lazr.restfulclient.errors import ServerError
 from sru_workflow import process_bug
@@ -364,8 +365,11 @@
     if debdiff and opts.view:
         with ExitStack() as resources:
             tfile = resources.enter_context(tempfile.NamedTemporaryFile())
- for line in debdiff:
- tfile.write(line)
+ while True:
+ for line in dropwhile(lambda x:not x.decode('utf-8').startswith('diff '), debdiff):
+ tfile.write(line)
+ else:
+ break
             tfile.flush()
             if opts.diffstat:
                 combinedfile = resources.enter_context(

Could you add that to your changes? Then I'll harass somebody who has commit access to this to get it merged. Thanks again!

review: Needs Fixing
1216. By Marco Trevisan (Treviño)

Merge with trunk

1217. By Brian Murray

sru-review: Remove the bileto redundant diffstat

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Ok, thanks... Added!

Revision history for this message
Brian Murray (brian-murray) :
review: Approve
Revision history for this message
Łukasz Zemczak (sil2100) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'sru-review'
2--- sru-review 2021-04-23 08:44:48 +0000
3+++ sru-review 2021-07-14 01:13:21 +0000
4@@ -35,13 +35,15 @@
5 import sys
6 import tempfile
7 try:
8- from urllib.parse import quote
9 from urllib.request import urlopen, urlretrieve
10+ from urllib.parse import quote, urlparse
11 except ImportError:
12 from urllib import urlopen, urlretrieve, quote
13+ from urlparse import urlparse
14 import webbrowser
15
16 from contextlib import ExitStack
17+from itertools import dropwhile
18 from launchpadlib.launchpad import Launchpad
19 from lazr.restfulclient.errors import ServerError
20 from sru_workflow import process_bug, check_additional_series
21@@ -142,8 +144,11 @@
22 sys.exit(1)
23 upload = uploads[0]
24
25+ is_bileto = False
26 if upload.contains_copy:
27 archive = upload.copy_source_archive
28+ is_bileto = archive.owner.name == 'ci-train-ppa-service' and \
29+ upload.display_arches == 'sync'
30 pubs = archive.getPublishedSources(
31 exact_match=True, source_name=upload.package_name,
32 version=upload.package_version)
33@@ -159,20 +164,30 @@
34 changes_url = upload.changes_file_url
35
36 if options.diff:
37- oops_re = re.compile('class="oopsid">(OOPS[a-zA-Z0-9-]+)<')
38- debdiff_re = re.compile(
39- 'href="(http://launchpadlibrarian.net/'
40- '\d+/%s_[^"_]+_[^_"]+\.diff\.gz)">\s*diff from' %
41- re.escape(sourcepkg))
42-
43- queue_html = urlopen(queue_url).read().decode('utf-8')
44-
45- m = oops_re.search(queue_html)
46- if m:
47- print('ERROR: Launchpad failure:', m.group(1), file=sys.stderr)
48- sys.exit(1)
49-
50- m = debdiff_re.search(queue_html)
51+ if is_bileto:
52+ archive = upload.copy_source_archive
53+ debdiff_page = archive.description
54+ debdiff_re = re.compile(
55+ '(https:\/\/objectstorage\.[-\w\d_.]+\.canonical\.com\/.*\/'
56+ 'bileto-%s\/[-\d_:]+\/%s_%s_content\.diff)' %
57+ (archive.name.split('.')[0],
58+ re.escape(options.release),
59+ re.escape(sourcepkg)))
60+ else:
61+ oops_re = re.compile('class="oopsid">(OOPS[a-zA-Z0-9-]+)<')
62+ debdiff_re = re.compile(
63+ 'href="(http://launchpadlibrarian.net/'
64+ '\d+/%s_[^"_]+_[^_"]+\.diff\.gz)">\s*diff from' %
65+ re.escape(sourcepkg))
66+
67+ debdiff_page = urlopen(queue_url).read().decode('utf-8')
68+
69+ m = oops_re.search(debdiff_page)
70+ if m:
71+ print('ERROR: Launchpad failure:', m.group(1), file=sys.stderr)
72+ sys.exit(1)
73+
74+ m = debdiff_re.search(debdiff_page)
75 if not m:
76 print('ERROR: queue does not have a debdiff', file=sys.stderr)
77 sys.exit(1)
78@@ -312,7 +327,11 @@
79 debdiff = None
80 if debdiff_url:
81 with tempfile.NamedTemporaryFile() as f:
82- debdiff = gzip.open(urlretrieve(debdiff_url, f.name)[0])
83+ urlretrieve(debdiff_url, f.name)
84+ if urlparse(debdiff_url).path.endswith('.gz'):
85+ debdiff = gzip.open(f.name)
86+ else:
87+ debdiff = open(f.name, 'rb')
88 elif opts.diff:
89 print('No debdiff available')
90
91@@ -346,8 +365,11 @@
92 if debdiff and opts.view:
93 with ExitStack() as resources:
94 tfile = resources.enter_context(tempfile.NamedTemporaryFile())
95- for line in debdiff:
96- tfile.write(line)
97+ while True:
98+ for line in dropwhile(lambda x:not x.decode('utf-8').startswith('diff '), debdiff):
99+ tfile.write(line)
100+ else:
101+ break
102 tfile.flush()
103 if opts.diffstat:
104 combinedfile = resources.enter_context(

Subscribers

People subscribed via source and target branches