Launchpad itself

Merge lp:~abrody/launchpad/https-mirror into lp:launchpad

  1. https-mirror
  2. Merge into devel
Proposed by Andy Brody
Status: Rejected
Rejected by: Colin Watson
Proposed branch: lp:~abrody/launchpad/https-mirror
Merge into: lp:launchpad
Diff against target: 272 lines (+55/-19)
9 files modified
lib/lp/registry/browser/distributionmirror.py (+6/-5)
lib/lp/registry/configure.zcml (+4/-2)
lib/lp/registry/interfaces/distribution.py (+3/-3)
lib/lp/registry/interfaces/distributionmirror.py (+17/-2)
lib/lp/registry/model/distribution.py (+6/-3)
lib/lp/registry/model/distributionmirror.py (+9/-1)
lib/lp/registry/templates/distributionmirror-index.pt (+4/-0)
lib/lp/registry/templates/distributionmirror-macros.pt (+3/-1)
lib/lp/testing/factory.py (+3/-2)
To merge this branch: bzr merge lp:~abrody/launchpad/https-mirror
Reviewer Review Type Date Requested Status
Colin Watson (community) Needs Fixing
Review via email: mp+362903@code.launchpad.net

This proposal supersedes a proposal from 2019-02-02.

Description of the change

This branch adds functionality to track HTTPS archive/CD mirror URLs.

It adds an https_base_url field much like the existing http_base_url field to the mirror create, edit, and list views.

lp:~abrody/launchpad/https-mirror-dbchange contains the DB changes.

$ make lint
= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  database/schema/patch-2209-99-0.sql
  lib/lp/registry/configure.zcml
  lib/lp/registry/browser/distributionmirror.py
  lib/lp/registry/interfaces/distribution.py
  lib/lp/registry/interfaces/distributionmirror.py
  lib/lp/registry/model/distribution.py
  lib/lp/registry/model/distributionmirror.py
  lib/lp/registry/templates/distributionmirror-index.pt
  lib/lp/registry/templates/distributionmirror-macros.pt
  lib/lp/testing/factory.py

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) wrote :

Thanks for working on this, and apologies for the delay in reviewing it.

This looks OK as far as it goes, but I'd expect some test changes as well. lib/lp/registry/stories/webservice/xx-distribution.txt and lib/lp/registry/stories/webservice/xx-distribution-mirror.txt will certainly need at least basic changes due to the newly-exported fields, and it would be good to add suitable tests to lib/lp/registry/browser/tests/distributionmirror-views.txt and lib/lp/registry/doc/distribution-mirror.txt.

Let us know if you have any difficulties running the test suite, and we can walk you through it.

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

Thiago picked this up in https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/379387, and it's now on production.

The Rejected status is a formality because this MP didn't land directly, but I wanted to say thanks for your work on this which did in fact make it into Launchpad in a slightly different form - it was a very useful starting point!

Unmerged revisions

18873. By Andy Brody

Add support for listing HTTPS archive/CD mirrors.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/browser/distributionmirror.py'
2--- lib/lp/registry/browser/distributionmirror.py 2015-10-13 13:22:08 +0000
3+++ lib/lp/registry/browser/distributionmirror.py 2019-04-13 16:32:28 +0000
4@@ -197,9 +197,9 @@
5 class DistributionMirrorAddView(LaunchpadFormView):
6 schema = IDistributionMirror
7 field_names = [
8- "display_name", "description", "whiteboard", "http_base_url",
9- "ftp_base_url", "rsync_base_url", "speed", "country", "content",
10- "official_candidate",
11+ "display_name", "description", "whiteboard", "https_base_url",
12+ "http_base_url", "ftp_base_url", "rsync_base_url", "speed", "country",
13+ "content", "official_candidate",
14 ]
15
16 @property
17@@ -224,6 +224,7 @@
18 content=data['content'], display_name=data['display_name'],
19 description=data['description'],
20 whiteboard=data['whiteboard'],
21+ https_base_url=data['https_base_url'],
22 http_base_url=data['http_base_url'],
23 ftp_base_url=data['ftp_base_url'],
24 rsync_base_url=data['rsync_base_url'],
25@@ -268,8 +269,8 @@
26 schema = IDistributionMirror
27 field_names = [
28 "name", "display_name", "description", "whiteboard",
29- "http_base_url", "ftp_base_url", "rsync_base_url", "speed",
30- "country", "content", "official_candidate",
31+ "https_base_url", "http_base_url", "ftp_base_url", "rsync_base_url",
32+ "speed", "country", "content", "official_candidate",
33 ]
34
35 @property
36
37=== modified file 'lib/lp/registry/configure.zcml'
38--- lib/lp/registry/configure.zcml 2018-04-22 23:30:37 +0000
39+++ lib/lp/registry/configure.zcml 2019-04-13 16:32:28 +0000
40@@ -1988,6 +1988,7 @@
41 description
42 distribution
43 http_base_url
44+ https_base_url
45 ftp_base_url
46 rsync_base_url
47 enabled
48@@ -2026,8 +2027,9 @@
49 <require
50 permission="launchpad.Edit"
51 set_attributes="name display_name description whiteboard
52- http_base_url ftp_base_url rsync_base_url enabled
53- speed country content official_candidate owner"
54+ http_base_url https_base_url ftp_base_url
55+ rsync_base_url enabled speed country content
56+ official_candidate owner"
57 attributes="official_candidate whiteboard" />
58 <require
59 permission="launchpad.Admin"
60
61=== modified file 'lib/lp/registry/interfaces/distribution.py'
62--- lib/lp/registry/interfaces/distribution.py 2015-10-13 13:22:08 +0000
63+++ lib/lp/registry/interfaces/distribution.py 2019-04-13 16:32:28 +0000
64@@ -491,13 +491,13 @@
65 """Return the country DNS mirror for a country and content type."""
66
67 def newMirror(owner, speed, country, content, display_name=None,
68- description=None, http_base_url=None,
69+ description=None, http_base_url=None, https_base_url=None,
70 ftp_base_url=None, rsync_base_url=None, enabled=False,
71 official_candidate=False, whiteboard=None):
72 """Create a new DistributionMirror for this distribution.
73
74- At least one of http_base_url or ftp_base_url must be provided in
75- order to create a mirror.
76+ At least one of {http,https,ftp}_base_url must be provided in order to
77+ create a mirror.
78 """
79
80 @operation_parameters(
81
82=== modified file 'lib/lp/registry/interfaces/distributionmirror.py'
83--- lib/lp/registry/interfaces/distributionmirror.py 2015-10-26 14:54:43 +0000
84+++ lib/lp/registry/interfaces/distributionmirror.py 2019-04-13 16:32:28 +0000
85@@ -295,6 +295,12 @@
86 return getUtility(IDistributionMirrorSet).getByHttpUrl(url)
87
88
89+class DistroMirrorHTTPSURIField(DistroMirrorURIField):
90+
91+ def getMirrorByURI(self, url):
92+ return getUtility(IDistributionMirrorSet).getByHttpsUrl(url)
93+
94+
95 class DistroMirrorFTPURIField(DistroMirrorURIField):
96
97 def getMirrorByURI(self, url):
98@@ -342,6 +348,11 @@
99 allowed_schemes=['http'], allow_userinfo=False,
100 allow_query=False, allow_fragment=False, trailing_slash=True,
101 description=_('e.g.: http://archive.ubuntu.com/ubuntu/')))
102+ https_base_url = exported(DistroMirrorHTTPSURIField(
103+ title=_('HTTPS URL'), required=False, readonly=False,
104+ allowed_schemes=['https'], allow_userinfo=False,
105+ allow_query=False, allow_fragment=False, trailing_slash=True,
106+ description=_('e.g.: https://archive.ubuntu.com/ubuntu/')))
107 ftp_base_url = exported(DistroMirrorFTPURIField(
108 title=_('FTP URL'), required=False, readonly=False,
109 allowed_schemes=['ftp'], allow_userinfo=False,
110@@ -428,8 +439,9 @@
111
112 @invariant
113 def mirrorMustHaveHTTPOrFTPURL(mirror):
114- if not (mirror.http_base_url or mirror.ftp_base_url):
115- raise Invalid('A mirror must have at least an HTTP or FTP URL.')
116+ if not (mirror.http_base_url or mirror.https_base_url or
117+ mirror.ftp_base_url):
118+ raise Invalid('A mirror must have at least an HTTP(S) or FTP URL.')
119
120 def getSummarizedMirroredSourceSeries():
121 """Return a summarized list of this distribution_mirror's
122@@ -601,6 +613,9 @@
123 def getByHttpUrl(url):
124 """Return the mirror with the given HTTP URL or None."""
125
126+ def getByHttpsUrl(url):
127+ """Return the mirror with the given HTTPS URL or None."""
128+
129 def getByFtpUrl(url):
130 """Return the mirror with the given FTP URL or None."""
131
132
133=== modified file 'lib/lp/registry/model/distribution.py'
134--- lib/lp/registry/model/distribution.py 2018-05-14 09:25:45 +0000
135+++ lib/lp/registry/model/distribution.py 2019-04-13 16:32:28 +0000
136@@ -708,7 +708,7 @@
137 country_dns_mirror=True).one()
138
139 def newMirror(self, owner, speed, country, content, display_name=None,
140- description=None, http_base_url=None,
141+ description=None, http_base_url=None, https_base_url=None,
142 ftp_base_url=None, rsync_base_url=None,
143 official_candidate=False, enabled=False,
144 whiteboard=None):
145@@ -721,15 +721,17 @@
146 return None
147
148 urls = {'http_base_url': http_base_url,
149+ 'https_base_url': https_base_url,
150 'ftp_base_url': ftp_base_url,
151 'rsync_base_url': rsync_base_url}
152 for name, value in urls.items():
153 if value is not None:
154 urls[name] = IDistributionMirror[name].normalize(value)
155
156- url = urls['http_base_url'] or urls['ftp_base_url']
157+ url = (urls['https_base_url'] or urls['http_base_url'] or
158+ urls['ftp_base_url'])
159 assert url is not None, (
160- "A mirror must provide either an HTTP or FTP URL (or both).")
161+ "A mirror must provide at least one HTTP/HTTPS/FTP URL.")
162 dummy, host, dummy, dummy, dummy, dummy = urlparse(url)
163 name = sanitize_name('%s-%s' % (host, content.name.lower()))
164
165@@ -743,6 +745,7 @@
166 distribution=self, owner=owner, name=name, speed=speed,
167 country=country, content=content, display_name=display_name,
168 description=description, http_base_url=urls['http_base_url'],
169+ https_base_url=urls['https_base_url'],
170 ftp_base_url=urls['ftp_base_url'],
171 rsync_base_url=urls['rsync_base_url'],
172 official_candidate=official_candidate, enabled=enabled,
173
174=== modified file 'lib/lp/registry/model/distributionmirror.py'
175--- lib/lp/registry/model/distributionmirror.py 2015-10-13 13:22:08 +0000
176+++ lib/lp/registry/model/distributionmirror.py 2019-04-13 16:32:28 +0000
177@@ -128,6 +128,8 @@
178 notNull=False, default=None)
179 http_base_url = StringCol(
180 notNull=False, default=None, unique=True)
181+ https_base_url = StringCol(
182+ notNull=False, default=None, unique=True)
183 ftp_base_url = StringCol(
184 notNull=False, default=None, unique=True)
185 rsync_base_url = StringCol(
186@@ -154,7 +156,9 @@
187 @property
188 def base_url(self):
189 """See IDistributionMirror"""
190- if self.http_base_url is not None:
191+ if self.https_base_url is not None:
192+ return self.https_base_url
193+ elif self.http_base_url is not None:
194 return self.http_base_url
195 else:
196 return self.ftp_base_url
197@@ -669,6 +673,10 @@
198 """See IDistributionMirrorSet"""
199 return DistributionMirror.selectOneBy(http_base_url=url)
200
201+ def getByHttpsUrl(self, url):
202+ """See IDistributionMirrorSet"""
203+ return DistributionMirror.selectOneBy(https_base_url=url)
204+
205 def getByFtpUrl(self, url):
206 """See IDistributionMirrorSet"""
207 return DistributionMirror.selectOneBy(ftp_base_url=url)
208
209=== modified file 'lib/lp/registry/templates/distributionmirror-index.pt'
210--- lib/lp/registry/templates/distributionmirror-index.pt 2010-10-10 21:54:16 +0000
211+++ lib/lp/registry/templates/distributionmirror-index.pt 2019-04-13 16:32:28 +0000
212@@ -118,6 +118,10 @@
213 <h2>Mirror location information</h2>
214
215 <ul class="webref" id="mirror-urls">
216+ <li tal:condition="context/https_base_url" >
217+ <a tal:content="context/https_base_url"
218+ tal:attributes="href context/https_base_url">https://url/</a>
219+ </li>
220 <li tal:condition="context/http_base_url" >
221 <a tal:content="context/http_base_url"
222 tal:attributes="href context/http_base_url">http://url/</a>
223
224=== modified file 'lib/lp/registry/templates/distributionmirror-macros.pt'
225--- lib/lp/registry/templates/distributionmirror-macros.pt 2015-02-18 18:44:32 +0000
226+++ lib/lp/registry/templates/distributionmirror-macros.pt 2019-04-13 16:32:28 +0000
227@@ -17,7 +17,7 @@
228 <tbody>
229 <tal:country_and_mirrors repeat="country_and_mirrors mirrors_by_country">
230 <tr class="head">
231- <th colspan="2"
232+ <th colspan="2"
233 tal:content="country_and_mirrors/country" />
234 <th tal:content="country_and_mirrors/throughput"/>
235 <th tal:condition="show_mirror_type">
236@@ -35,6 +35,8 @@
237 tal:content="mirror/title">Mirror Name</a>
238 </td>
239 <td>
240+ <a tal:condition="mirror/https_base_url"
241+ tal:attributes="href mirror/https_base_url">https</a>
242 <a tal:condition="mirror/http_base_url"
243 tal:attributes="href mirror/http_base_url">http</a>
244 <a tal:condition="mirror/ftp_base_url"
245
246=== modified file 'lib/lp/testing/factory.py'
247--- lib/lp/testing/factory.py 2019-02-07 15:04:13 +0000
248+++ lib/lp/testing/factory.py 2019-04-13 16:32:28 +0000
249@@ -3516,13 +3516,13 @@
250 return proberecord
251
252 def makeMirror(self, distribution, displayname=None, country=None,
253- http_url=None, ftp_url=None, rsync_url=None,
254+ http_url=None, https_url=None, ftp_url=None, rsync_url=None,
255 official_candidate=False):
256 """Create a mirror for the distribution."""
257 if displayname is None:
258 displayname = self.getUniqueString("mirror")
259 # If no URL is specified create an HTTP URL.
260- if http_url is None and ftp_url is None and rsync_url is None:
261+ if http_url is https_url is ftp_url is rsync_url is None:
262 http_url = self.getUniqueURL()
263 # If no country is given use Argentina.
264 if country is None:
265@@ -3536,6 +3536,7 @@
266 display_name=displayname,
267 description=None,
268 http_base_url=http_url,
269+ https_base_url=https_url,
270 ftp_base_url=ftp_url,
271 rsync_base_url=rsync_url,
272 official_candidate=official_candidate)