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

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
=== modified file 'lib/lp/registry/browser/distributionmirror.py'
--- lib/lp/registry/browser/distributionmirror.py 2015-10-13 13:22:08 +0000
+++ lib/lp/registry/browser/distributionmirror.py 2019-04-13 16:32:28 +0000
@@ -197,9 +197,9 @@
197class DistributionMirrorAddView(LaunchpadFormView):197class DistributionMirrorAddView(LaunchpadFormView):
198 schema = IDistributionMirror198 schema = IDistributionMirror
199 field_names = [199 field_names = [
200 "display_name", "description", "whiteboard", "http_base_url",200 "display_name", "description", "whiteboard", "https_base_url",
201 "ftp_base_url", "rsync_base_url", "speed", "country", "content",201 "http_base_url", "ftp_base_url", "rsync_base_url", "speed", "country",
202 "official_candidate",202 "content", "official_candidate",
203 ]203 ]
204204
205 @property205 @property
@@ -224,6 +224,7 @@
224 content=data['content'], display_name=data['display_name'],224 content=data['content'], display_name=data['display_name'],
225 description=data['description'],225 description=data['description'],
226 whiteboard=data['whiteboard'],226 whiteboard=data['whiteboard'],
227 https_base_url=data['https_base_url'],
227 http_base_url=data['http_base_url'],228 http_base_url=data['http_base_url'],
228 ftp_base_url=data['ftp_base_url'],229 ftp_base_url=data['ftp_base_url'],
229 rsync_base_url=data['rsync_base_url'],230 rsync_base_url=data['rsync_base_url'],
@@ -268,8 +269,8 @@
268 schema = IDistributionMirror269 schema = IDistributionMirror
269 field_names = [270 field_names = [
270 "name", "display_name", "description", "whiteboard",271 "name", "display_name", "description", "whiteboard",
271 "http_base_url", "ftp_base_url", "rsync_base_url", "speed",272 "https_base_url", "http_base_url", "ftp_base_url", "rsync_base_url",
272 "country", "content", "official_candidate",273 "speed", "country", "content", "official_candidate",
273 ]274 ]
274275
275 @property276 @property
276277
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2018-04-22 23:30:37 +0000
+++ lib/lp/registry/configure.zcml 2019-04-13 16:32:28 +0000
@@ -1988,6 +1988,7 @@
1988 description1988 description
1989 distribution1989 distribution
1990 http_base_url1990 http_base_url
1991 https_base_url
1991 ftp_base_url1992 ftp_base_url
1992 rsync_base_url1993 rsync_base_url
1993 enabled1994 enabled
@@ -2026,8 +2027,9 @@
2026 <require2027 <require
2027 permission="launchpad.Edit"2028 permission="launchpad.Edit"
2028 set_attributes="name display_name description whiteboard2029 set_attributes="name display_name description whiteboard
2029 http_base_url ftp_base_url rsync_base_url enabled2030 http_base_url https_base_url ftp_base_url
2030 speed country content official_candidate owner"2031 rsync_base_url enabled speed country content
2032 official_candidate owner"
2031 attributes="official_candidate whiteboard" />2033 attributes="official_candidate whiteboard" />
2032 <require2034 <require
2033 permission="launchpad.Admin"2035 permission="launchpad.Admin"
20342036
=== modified file 'lib/lp/registry/interfaces/distribution.py'
--- lib/lp/registry/interfaces/distribution.py 2015-10-13 13:22:08 +0000
+++ lib/lp/registry/interfaces/distribution.py 2019-04-13 16:32:28 +0000
@@ -491,13 +491,13 @@
491 """Return the country DNS mirror for a country and content type."""491 """Return the country DNS mirror for a country and content type."""
492492
493 def newMirror(owner, speed, country, content, display_name=None,493 def newMirror(owner, speed, country, content, display_name=None,
494 description=None, http_base_url=None,494 description=None, http_base_url=None, https_base_url=None,
495 ftp_base_url=None, rsync_base_url=None, enabled=False,495 ftp_base_url=None, rsync_base_url=None, enabled=False,
496 official_candidate=False, whiteboard=None):496 official_candidate=False, whiteboard=None):
497 """Create a new DistributionMirror for this distribution.497 """Create a new DistributionMirror for this distribution.
498498
499 At least one of http_base_url or ftp_base_url must be provided in499 At least one of {http,https,ftp}_base_url must be provided in order to
500 order to create a mirror.500 create a mirror.
501 """501 """
502502
503 @operation_parameters(503 @operation_parameters(
504504
=== modified file 'lib/lp/registry/interfaces/distributionmirror.py'
--- lib/lp/registry/interfaces/distributionmirror.py 2015-10-26 14:54:43 +0000
+++ lib/lp/registry/interfaces/distributionmirror.py 2019-04-13 16:32:28 +0000
@@ -295,6 +295,12 @@
295 return getUtility(IDistributionMirrorSet).getByHttpUrl(url)295 return getUtility(IDistributionMirrorSet).getByHttpUrl(url)
296296
297297
298class DistroMirrorHTTPSURIField(DistroMirrorURIField):
299
300 def getMirrorByURI(self, url):
301 return getUtility(IDistributionMirrorSet).getByHttpsUrl(url)
302
303
298class DistroMirrorFTPURIField(DistroMirrorURIField):304class DistroMirrorFTPURIField(DistroMirrorURIField):
299305
300 def getMirrorByURI(self, url):306 def getMirrorByURI(self, url):
@@ -342,6 +348,11 @@
342 allowed_schemes=['http'], allow_userinfo=False,348 allowed_schemes=['http'], allow_userinfo=False,
343 allow_query=False, allow_fragment=False, trailing_slash=True,349 allow_query=False, allow_fragment=False, trailing_slash=True,
344 description=_('e.g.: http://archive.ubuntu.com/ubuntu/')))350 description=_('e.g.: http://archive.ubuntu.com/ubuntu/')))
351 https_base_url = exported(DistroMirrorHTTPSURIField(
352 title=_('HTTPS URL'), required=False, readonly=False,
353 allowed_schemes=['https'], allow_userinfo=False,
354 allow_query=False, allow_fragment=False, trailing_slash=True,
355 description=_('e.g.: https://archive.ubuntu.com/ubuntu/')))
345 ftp_base_url = exported(DistroMirrorFTPURIField(356 ftp_base_url = exported(DistroMirrorFTPURIField(
346 title=_('FTP URL'), required=False, readonly=False,357 title=_('FTP URL'), required=False, readonly=False,
347 allowed_schemes=['ftp'], allow_userinfo=False,358 allowed_schemes=['ftp'], allow_userinfo=False,
@@ -428,8 +439,9 @@
428439
429 @invariant440 @invariant
430 def mirrorMustHaveHTTPOrFTPURL(mirror):441 def mirrorMustHaveHTTPOrFTPURL(mirror):
431 if not (mirror.http_base_url or mirror.ftp_base_url):442 if not (mirror.http_base_url or mirror.https_base_url or
432 raise Invalid('A mirror must have at least an HTTP or FTP URL.')443 mirror.ftp_base_url):
444 raise Invalid('A mirror must have at least an HTTP(S) or FTP URL.')
433445
434 def getSummarizedMirroredSourceSeries():446 def getSummarizedMirroredSourceSeries():
435 """Return a summarized list of this distribution_mirror's447 """Return a summarized list of this distribution_mirror's
@@ -601,6 +613,9 @@
601 def getByHttpUrl(url):613 def getByHttpUrl(url):
602 """Return the mirror with the given HTTP URL or None."""614 """Return the mirror with the given HTTP URL or None."""
603615
616 def getByHttpsUrl(url):
617 """Return the mirror with the given HTTPS URL or None."""
618
604 def getByFtpUrl(url):619 def getByFtpUrl(url):
605 """Return the mirror with the given FTP URL or None."""620 """Return the mirror with the given FTP URL or None."""
606621
607622
=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py 2018-05-14 09:25:45 +0000
+++ lib/lp/registry/model/distribution.py 2019-04-13 16:32:28 +0000
@@ -708,7 +708,7 @@
708 country_dns_mirror=True).one()708 country_dns_mirror=True).one()
709709
710 def newMirror(self, owner, speed, country, content, display_name=None,710 def newMirror(self, owner, speed, country, content, display_name=None,
711 description=None, http_base_url=None,711 description=None, http_base_url=None, https_base_url=None,
712 ftp_base_url=None, rsync_base_url=None,712 ftp_base_url=None, rsync_base_url=None,
713 official_candidate=False, enabled=False,713 official_candidate=False, enabled=False,
714 whiteboard=None):714 whiteboard=None):
@@ -721,15 +721,17 @@
721 return None721 return None
722722
723 urls = {'http_base_url': http_base_url,723 urls = {'http_base_url': http_base_url,
724 'https_base_url': https_base_url,
724 'ftp_base_url': ftp_base_url,725 'ftp_base_url': ftp_base_url,
725 'rsync_base_url': rsync_base_url}726 'rsync_base_url': rsync_base_url}
726 for name, value in urls.items():727 for name, value in urls.items():
727 if value is not None:728 if value is not None:
728 urls[name] = IDistributionMirror[name].normalize(value)729 urls[name] = IDistributionMirror[name].normalize(value)
729730
730 url = urls['http_base_url'] or urls['ftp_base_url']731 url = (urls['https_base_url'] or urls['http_base_url'] or
732 urls['ftp_base_url'])
731 assert url is not None, (733 assert url is not None, (
732 "A mirror must provide either an HTTP or FTP URL (or both).")734 "A mirror must provide at least one HTTP/HTTPS/FTP URL.")
733 dummy, host, dummy, dummy, dummy, dummy = urlparse(url)735 dummy, host, dummy, dummy, dummy, dummy = urlparse(url)
734 name = sanitize_name('%s-%s' % (host, content.name.lower()))736 name = sanitize_name('%s-%s' % (host, content.name.lower()))
735737
@@ -743,6 +745,7 @@
743 distribution=self, owner=owner, name=name, speed=speed,745 distribution=self, owner=owner, name=name, speed=speed,
744 country=country, content=content, display_name=display_name,746 country=country, content=content, display_name=display_name,
745 description=description, http_base_url=urls['http_base_url'],747 description=description, http_base_url=urls['http_base_url'],
748 https_base_url=urls['https_base_url'],
746 ftp_base_url=urls['ftp_base_url'],749 ftp_base_url=urls['ftp_base_url'],
747 rsync_base_url=urls['rsync_base_url'],750 rsync_base_url=urls['rsync_base_url'],
748 official_candidate=official_candidate, enabled=enabled,751 official_candidate=official_candidate, enabled=enabled,
749752
=== modified file 'lib/lp/registry/model/distributionmirror.py'
--- lib/lp/registry/model/distributionmirror.py 2015-10-13 13:22:08 +0000
+++ lib/lp/registry/model/distributionmirror.py 2019-04-13 16:32:28 +0000
@@ -128,6 +128,8 @@
128 notNull=False, default=None)128 notNull=False, default=None)
129 http_base_url = StringCol(129 http_base_url = StringCol(
130 notNull=False, default=None, unique=True)130 notNull=False, default=None, unique=True)
131 https_base_url = StringCol(
132 notNull=False, default=None, unique=True)
131 ftp_base_url = StringCol(133 ftp_base_url = StringCol(
132 notNull=False, default=None, unique=True)134 notNull=False, default=None, unique=True)
133 rsync_base_url = StringCol(135 rsync_base_url = StringCol(
@@ -154,7 +156,9 @@
154 @property156 @property
155 def base_url(self):157 def base_url(self):
156 """See IDistributionMirror"""158 """See IDistributionMirror"""
157 if self.http_base_url is not None:159 if self.https_base_url is not None:
160 return self.https_base_url
161 elif self.http_base_url is not None:
158 return self.http_base_url162 return self.http_base_url
159 else:163 else:
160 return self.ftp_base_url164 return self.ftp_base_url
@@ -669,6 +673,10 @@
669 """See IDistributionMirrorSet"""673 """See IDistributionMirrorSet"""
670 return DistributionMirror.selectOneBy(http_base_url=url)674 return DistributionMirror.selectOneBy(http_base_url=url)
671675
676 def getByHttpsUrl(self, url):
677 """See IDistributionMirrorSet"""
678 return DistributionMirror.selectOneBy(https_base_url=url)
679
672 def getByFtpUrl(self, url):680 def getByFtpUrl(self, url):
673 """See IDistributionMirrorSet"""681 """See IDistributionMirrorSet"""
674 return DistributionMirror.selectOneBy(ftp_base_url=url)682 return DistributionMirror.selectOneBy(ftp_base_url=url)
675683
=== modified file 'lib/lp/registry/templates/distributionmirror-index.pt'
--- lib/lp/registry/templates/distributionmirror-index.pt 2010-10-10 21:54:16 +0000
+++ lib/lp/registry/templates/distributionmirror-index.pt 2019-04-13 16:32:28 +0000
@@ -118,6 +118,10 @@
118 <h2>Mirror location information</h2>118 <h2>Mirror location information</h2>
119119
120 <ul class="webref" id="mirror-urls">120 <ul class="webref" id="mirror-urls">
121 <li tal:condition="context/https_base_url" >
122 <a tal:content="context/https_base_url"
123 tal:attributes="href context/https_base_url">https://url/</a>
124 </li>
121 <li tal:condition="context/http_base_url" >125 <li tal:condition="context/http_base_url" >
122 <a tal:content="context/http_base_url"126 <a tal:content="context/http_base_url"
123 tal:attributes="href context/http_base_url">http://url/</a>127 tal:attributes="href context/http_base_url">http://url/</a>
124128
=== modified file 'lib/lp/registry/templates/distributionmirror-macros.pt'
--- lib/lp/registry/templates/distributionmirror-macros.pt 2015-02-18 18:44:32 +0000
+++ lib/lp/registry/templates/distributionmirror-macros.pt 2019-04-13 16:32:28 +0000
@@ -17,7 +17,7 @@
17 <tbody>17 <tbody>
18 <tal:country_and_mirrors repeat="country_and_mirrors mirrors_by_country">18 <tal:country_and_mirrors repeat="country_and_mirrors mirrors_by_country">
19 <tr class="head">19 <tr class="head">
20 <th colspan="2" 20 <th colspan="2"
21 tal:content="country_and_mirrors/country" />21 tal:content="country_and_mirrors/country" />
22 <th tal:content="country_and_mirrors/throughput"/>22 <th tal:content="country_and_mirrors/throughput"/>
23 <th tal:condition="show_mirror_type">23 <th tal:condition="show_mirror_type">
@@ -35,6 +35,8 @@
35 tal:content="mirror/title">Mirror Name</a>35 tal:content="mirror/title">Mirror Name</a>
36 </td>36 </td>
37 <td>37 <td>
38 <a tal:condition="mirror/https_base_url"
39 tal:attributes="href mirror/https_base_url">https</a>
38 <a tal:condition="mirror/http_base_url"40 <a tal:condition="mirror/http_base_url"
39 tal:attributes="href mirror/http_base_url">http</a>41 tal:attributes="href mirror/http_base_url">http</a>
40 <a tal:condition="mirror/ftp_base_url"42 <a tal:condition="mirror/ftp_base_url"
4143
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2019-02-07 15:04:13 +0000
+++ lib/lp/testing/factory.py 2019-04-13 16:32:28 +0000
@@ -3516,13 +3516,13 @@
3516 return proberecord3516 return proberecord
35173517
3518 def makeMirror(self, distribution, displayname=None, country=None,3518 def makeMirror(self, distribution, displayname=None, country=None,
3519 http_url=None, ftp_url=None, rsync_url=None,3519 http_url=None, https_url=None, ftp_url=None, rsync_url=None,
3520 official_candidate=False):3520 official_candidate=False):
3521 """Create a mirror for the distribution."""3521 """Create a mirror for the distribution."""
3522 if displayname is None:3522 if displayname is None:
3523 displayname = self.getUniqueString("mirror")3523 displayname = self.getUniqueString("mirror")
3524 # If no URL is specified create an HTTP URL.3524 # If no URL is specified create an HTTP URL.
3525 if http_url is None and ftp_url is None and rsync_url is None:3525 if http_url is https_url is ftp_url is rsync_url is None:
3526 http_url = self.getUniqueURL()3526 http_url = self.getUniqueURL()
3527 # If no country is given use Argentina.3527 # If no country is given use Argentina.
3528 if country is None:3528 if country is None:
@@ -3536,6 +3536,7 @@
3536 display_name=displayname,3536 display_name=displayname,
3537 description=None,3537 description=None,
3538 http_base_url=http_url,3538 http_base_url=http_url,
3539 https_base_url=https_url,
3539 ftp_base_url=ftp_url,3540 ftp_base_url=ftp_url,
3540 rsync_base_url=rsync_url,3541 rsync_base_url=rsync_url,
3541 official_candidate=official_candidate)3542 official_candidate=official_candidate)