Merge lp:~seb128/ubuntu-archive-scripts/verdict-not-package into lp:ubuntu-archive-scripts

Proposed by Sebastien Bacher
Status: Merged
Merged at revision: 282
Proposed branch: lp:~seb128/ubuntu-archive-scripts/verdict-not-package
Merge into: lp:ubuntu-archive-scripts
Diff against target: 45 lines (+14/-2)
1 file modified
generate-team-p-m (+14/-2)
To merge this branch: bzr merge lp:~seb128/ubuntu-archive-scripts/verdict-not-package
Reviewer Review Type Date Requested Status
Iain Lane Approve
Ubuntu Package Archive Administrators Pending
Review via email: mp+386000@code.launchpad.net

Commit message

the 'verdict" entries in the autopkgtest sections are not packages

fix the code with the new britney output

Description of the change

Using the new britney version the yaml has such entries

    autopkgtest:
      verdict: REJECTED_TEMPORARILY

or the code assumed the autopkgtest section only includes packages entries and hits an error

Traceback (most recent call last):
  File "./generate-team-p-m", line 352, in <module>
    main()
  File "./generate-team-p-m", line 265, in main
    for arch, result in sorted(results.items()):
AttributeError: 'str' object has no attribute 'items'

ignoring those lines makes the report work again

To post a comment you must log in.
Revision history for this message
Iain Lane (laney) wrote :

Thanks.

Just wanted to say that I'm talking to upstream about restructuring the YAML due to this (it would be unfortunate if we ever get a package called 'verdict'), so the fix required might look a bit different in the end.

review: Abstain
Revision history for this message
Iain Lane (laney) wrote :

Looks good but isn't this also a problem for update-excuse and block-bugs bugs too?

review: Approve
Revision history for this message
Iain Lane (laney) wrote :

(the restructuring I mentioned will probably happen but not right now)

283. By Sebastien Bacher

ignore verdict entries for block-bug and update-excuse

Revision history for this message
Sebastien Bacher (seb128) wrote :

thanks for reviewing!

> Looks good but isn't this also a problem for update-excuse and block-bugs bugs too?

it is but the fix was a bit less trivial since those sections are handled in the template and not in the code, I've pushed extra changes that should do the job

Revision history for this message
Iain Lane (laney) wrote :

Ah yeah, this works I guess but you could also delete those entries before handing the dictionary to the template, that might be cleaner? up to you, something like this (not tested!):

=== modified file 'generate-team-p-m'
--- old/generate-team-p-m 2020-04-07 10:02:51 +0000
+++ new/generate-team-p-m 2020-06-30 15:06:56 +0000
@@ -256,6 +256,11 @@
         in_proposed_packages[source_package_name] = prob
         prob.regressions = []
         prob.waiting = []
+ for policy in ['autopkgtest', 'update-excuse', 'block-bugs']:
+ try:
+ del item['policy_info'][policy]['verdict']
+ except KeyError:
+ pass
         if 'autopkgtest' in item['reason']:
             for package, results in sorted(item['policy_info']['autopkgtest'].items()):
                 regr_arches = []

Revision history for this message
Iain Lane (laney) wrote :

bah, Launchpad messed up my indentation there, hopefully you get the idea

284. By Sebastien Bacher

exclude the verdit entries from the code it's simpler, thanks Laney!

285. By Sebastien Bacher

simply the template code again

286. By Sebastien Bacher

try reading the xz report first, if not fallback to the uncompressed one

287. By Sebastien Bacher

there is no need to read the report before iterating

288. By Sebastien Bacher

specify the urllib error to catch

289. By Sebastien Bacher

indicate when the fallback url is read and display the error

290. By Sebastien Bacher

don't special case verdicts entries, they are filter out now

Revision history for this message
Iain Lane (laney) wrote :

lgtm

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'generate-team-p-m'
2--- generate-team-p-m 2020-04-07 10:02:51 +0000
3+++ generate-team-p-m 2020-07-01 08:20:30 +0000
4@@ -23,10 +23,12 @@
5 import os
6 import threading
7 from urllib.request import urlopen
8+import urllib.error
9
10 import attr
11 from jinja2 import Environment, FileSystemLoader
12 import yaml
13+import lzma
14
15 env = Environment(
16 loader=FileSystemLoader(os.path.dirname(os.path.abspath(__file__)) + '/templates'),
17@@ -238,9 +240,13 @@
18
19 print("fetching yaml")
20 if args.excuses_yaml:
21- yaml_text = open(args.excuses_yaml).read()
22+ yaml_text = open(args.excuses_yaml)
23 else:
24- yaml_text = urlopen("https://people.canonical.com/~ubuntu-archive/proposed-migration/update_excuses.yaml").read()
25+ try:
26+ yaml_text = lzma.open(urlopen("https://people.canonical.com/~ubuntu-archive/proposed-migration/update_excuses.yaml.xz"))
27+ except urllib.error.HTTPError as e:
28+ print("Reading fallback yaml (%s)" % e)
29+ yaml_text = urlopen("https://people.canonical.com/~ubuntu-archive/proposed-migration/update_excuses.yaml")
30 print("parsing yaml")
31 # The CSafeLoader is ten times faster than the regular one
32 excuses = yaml.load(yaml_text, Loader=yaml.CSafeLoader)
33@@ -256,6 +262,12 @@
34 in_proposed_packages[source_package_name] = prob
35 prob.regressions = []
36 prob.waiting = []
37+ # The verdict entries are not items to list on the report
38+ for policy in ['autopkgtest', 'update-excuse', 'block-bugs']:
39+ try:
40+ del item['policy_info'][policy]['verdict']
41+ except KeyError:
42+ pass
43 if 'autopkgtest' in item['reason']:
44 for package, results in sorted(item['policy_info']['autopkgtest'].items()):
45 regr_arches = []

Subscribers

People subscribed via source and target branches