Merge ~hyask/autopkgtest-cloud:skia/package_link into autopkgtest-cloud:master

Proposed by Skia
Status: Merged
Merged at revision: 89bec7a60a6896da56562d1ca0c83f3e6a2c4f34
Proposed branch: ~hyask/autopkgtest-cloud:skia/package_link
Merge into: autopkgtest-cloud:master
Diff against target: 76 lines (+24/-0)
4 files modified
charms/focal/autopkgtest-web/webcontrol/helpers/tests.py (+2/-0)
charms/focal/autopkgtest-web/webcontrol/templates/browse-package.html (+3/-0)
charms/focal/autopkgtest-web/webcontrol/templates/browse-results.html (+5/-0)
charms/focal/autopkgtest-web/webcontrol/templates/macros.html (+14/-0)
Reviewer Review Type Date Requested Status
Tim Andersson Approve
Review via email: mp+461440@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Tim Andersson (andersson123) wrote :

One "Needs fixing":
- I think the first commit message, "helper" isn't too clear. Change to "helpers" and also mention the commit affects tests.py - It's not exactly a helper function we use in the actual infra so I think that should be specified, otherwise the commit message isn't exactly descriptive of the change

One "Nice to have":
- Is there a way for us to check if a "generic" launchpad url like [1] is valid for a specified package and only add the hyperlink if the page is valid? I just think even though it makes the change a bit more complicated, why not add it now if we can :P

[1] https://bugs.launchpad.net/auto-package-testing

review: Needs Fixing
Revision history for this message
Skia (hyask) wrote :

Fixed the commit message, thanks, it's changed.

For the nice to have, there is two problems:
  * it's really hard to verify from a Jinja template that the link is valid: we would have to perform the check from Python and generate the link there, but that feel absolutely overkill for what we want.
  * `auto-package-testing` is not a package, it's a project, so it doesn't make sense to make a link pointing there, where would it be? Sometimes, both links exist [1], but given that we're linking from autopkgtest, the package page makes more sense than the project page, I think.

[1]: see curtin, for example:
project: https://launchpad.net/curtin
package: https://launchpad.net/ubuntu/+source/curtin

Revision history for this message
Tim Andersson (andersson123) wrote :

Yes, I believe the first bullet point you mention is the only way to go about it. I think it's something we should think about and maybe do in the future.

What about checking a URL like this:
https://bugs.launchpad.net/ubuntu/+source/curtin

?

Package definitely makes more sense than project page though.

If you want to implement the bug link as well, go ahead. Otherwise, this LGTM!

review: Approve
Revision history for this message
Brian Murray (brian-murray) :
Revision history for this message
Skia (hyask) :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/charms/focal/autopkgtest-web/webcontrol/helpers/tests.py b/charms/focal/autopkgtest-web/webcontrol/helpers/tests.py
2index 504e855..de23699 100644
3--- a/charms/focal/autopkgtest-web/webcontrol/helpers/tests.py
4+++ b/charms/focal/autopkgtest-web/webcontrol/helpers/tests.py
5@@ -19,6 +19,7 @@ def populate_dummy_db(db_con):
6 (7, supported_releases[2], "arm64", "hello2"),
7 (8, supported_releases[3], "amd64", "hello2"),
8 (9, supported_releases[3], "arm64", "hello2"),
9+ (10, supported_releases[0], "amd64", "vim"),
10 ]
11 c.executemany("INSERT INTO test values(?, ?, ?, ?)", tests)
12 results = [
13@@ -34,6 +35,7 @@ def populate_dummy_db(db_con):
14 (7, datetime.now(), "2.0.0", "hello/1.2.3", 142, 16, "", "", str(uuid4())),
15 (8, datetime.now(), "2.0.0", "hello/1.2.3", 142, 20, "", "", str(uuid4())),
16 (9, datetime.now(), "2.0.0", "hello/1.2.3", 142, 0, "", "", str(uuid4())),
17+ (10, datetime.now(), "2:9.1.0016-1", "vim/2:9.1.0016-1", 1142, 0, "", "", str(uuid4())),
18 # fmt: on
19 ]
20 c.executemany(
21diff --git a/charms/focal/autopkgtest-web/webcontrol/static/launchpad.ico b/charms/focal/autopkgtest-web/webcontrol/static/launchpad.ico
22new file mode 100644
23index 0000000..8c4c834
24Binary files /dev/null and b/charms/focal/autopkgtest-web/webcontrol/static/launchpad.ico differ
25diff --git a/charms/focal/autopkgtest-web/webcontrol/templates/browse-package.html b/charms/focal/autopkgtest-web/webcontrol/templates/browse-package.html
26index 65d14de..ea59f0f 100644
27--- a/charms/focal/autopkgtest-web/webcontrol/templates/browse-package.html
28+++ b/charms/focal/autopkgtest-web/webcontrol/templates/browse-package.html
29@@ -3,6 +3,9 @@
30
31 {% block content %}
32 <h2>{{package}}</h2>
33+ {{ macros.launchpad_link(package) }}
34+ |
35+ {{ macros.excuses_link(package) }}
36
37 <table class="table" style='width: auto'>
38 <tr>
39diff --git a/charms/focal/autopkgtest-web/webcontrol/templates/browse-results.html b/charms/focal/autopkgtest-web/webcontrol/templates/browse-results.html
40index fd2332a..fd300f7 100644
41--- a/charms/focal/autopkgtest-web/webcontrol/templates/browse-results.html
42+++ b/charms/focal/autopkgtest-web/webcontrol/templates/browse-results.html
43@@ -1,6 +1,11 @@
44 {% extends "browse-layout.html" %}
45+{% import "macros.html" as macros %}
46+
47 {% block content %}
48 <h2><a href="{{base_url}}packages/{{package}}">{{package}}</a> <small>[{{release}}/{{arch}}]</small></h2>
49+ {{ macros.launchpad_link(package, release) }}
50+ |
51+ {{ macros.excuses_link(package, release) }}
52
53 <table class="table">
54 <tr>
55diff --git a/charms/focal/autopkgtest-web/webcontrol/templates/macros.html b/charms/focal/autopkgtest-web/webcontrol/templates/macros.html
56index 77400c8..8995301 100644
57--- a/charms/focal/autopkgtest-web/webcontrol/templates/macros.html
58+++ b/charms/focal/autopkgtest-web/webcontrol/templates/macros.html
59@@ -48,3 +48,17 @@
60 {%- endfor %}
61 {%- endfor %}
62 {%- endmacro %}
63+
64+{% macro excuses_link(package_name, release="") -%}
65+{% if release != "" %}
66+{% set release = release + "/" %}
67+{% endif %}
68+<a href="https://ubuntu-archive-team.ubuntu.com/proposed-migration/{{ release }}update_excuses.html#{{ package_name }}">excuses</a>
69+{%- endmacro %}
70+
71+{% macro launchpad_link(package_name, release="") -%}
72+{% if release != "" %}
73+{% set release = release + "/" %}
74+{% endif %}
75+<a href="https://launchpad.net/ubuntu/{{ release }}+source/{{ package_name }}"><img src="{{ url_for('static', filename='launchpad.ico') }}" style="height: 1em;">Launchpad</a>
76+{%- endmacro %}

Subscribers

People subscribed via source and target branches