Launchpad itself

Merge lp:~jpds/launchpad/fix_361650 into lp:launchpad

  1. fix_361650
  2. Merge into devel
Proposed by Jonathan Davies
Status: Rejected
Rejected by: Curtis Hovey
Proposed branch: lp:~jpds/launchpad/fix_361650
Merge into: lp:launchpad
Diff against target: 545 lines (+306/-19)
12 files modified
database/sampledata/current-dev.sql (+6/-0)
database/sampledata/current.sql (+6/-0)
database/schema/comments.sql (+1/-0)
database/schema/patch-2207-20-0.sql (+19/-0)
lib/lp/registry/browser/distributionmirror.py (+63/-4)
lib/lp/registry/browser/tests/distributionmirror-views.txt (+1/-1)
lib/lp/registry/configure.zcml (+1/-1)
lib/lp/registry/interfaces/distributionmirror.py (+9/-1)
lib/lp/registry/model/distributionmirror.py (+22/-0)
lib/lp/registry/stories/distributionmirror/xx-distribution-mirrors.txt (+162/-11)
lib/lp/registry/templates/distributionmirror-index.pt (+9/-0)
lib/lp/registry/templates/distributionmirror-macros.pt (+7/-1)
To merge this branch: bzr merge lp:~jpds/launchpad/fix_361650
Reviewer Review Type Date Requested Status
Curtis Hovey (community) ui and code Needs Fixing
Jonathan Lange (community) db Approve
Michael Nelson (community) ui Needs Information
Stuart Bishop (community) db Approve
Canonical Launchpad Engineering code Pending
Review via email: mp+16749@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Jonathan Davies (jpds) wrote :

= Summary =

Launchpad should know which mirrors are set as official country mirrors (eg. gb.archive.ubuntu.com), so that we can track these easily on mirror listings.

This branch also includes a database change (patch-2207-20-0.sql) which adds a official_country_mirror column to distributionmirror. It also includes two constraints to ensure that there can not be more than one (archive|releases) mirror per country.

New mirrors have been added to current(-dev).sql for testing purposes too.

A mirror should only be eligible for country mirror status when:

1) it has been reviewed and set as an official mirror by an admin,
2) it has been probed for integrity,
3) it has an HTTP URL set.
4) there is no country mirror for that mirror type in the country.

Checks have been added to the code to ensure that these conditions are met.

Revision history for this message
Jonathan Lange (jml) wrote :

Hello Jonathan,

Thanks for this patch -- I can see how this would be useful.

I don't have any serious issues with the database patch -- the uniqueness constraint looks correct to me. My main concern is that using the word 'official' to describe this new concept will create confusion with the already existing concept of 'official mirror'. Would 'primary_mirror' make sense?

Other than that, the db patch looks good to me.

jml

review: Needs Fixing (db)
Revision history for this message
Stuart Bishop (stub) wrote :

I don't much like the column name either, but it does match the domain language used at http://www.ubuntu.com/getubuntu/mirror/1 so I guess it is correct.

Why are we limiting things to one official country mirror per country? Is there a technical issue we need to ensure doesn't happen, or is this an arbitrary choice made by us or the mirror maintainers?

Patch number is patch-2207-23-0.sql. Its good to land if you can confirm that the UNIQUE constraints are actually desirable.

review: Approve (db)
Revision history for this message
Stuart Bishop (stub) wrote :

> I don't much like the column name either, but it does match the domain
> language used at http://www.ubuntu.com/getubuntu/mirror/1 so I guess it is
> correct.
>
> Why are we limiting things to one official country mirror per country? Is
> there a technical issue we need to ensure doesn't happen, or is this an
> arbitrary choice made by us or the mirror maintainers?
>
> Patch number is patch-2207-23-0.sql. Its good to land if you can confirm that
> the UNIQUE constraints are actually desirable.

One thing that needs changing - You should use 'IS TRUE' and 'IS FALSE' rather than '= TRUE' or '= FALSE'. = and IS give different results in SQL's three valued boolean arithmetic which can confuse the query planner since it isn't as smart as you.

Revision history for this message
Jonathan Davies (jpds) wrote :

Morning Jonathan,

Country mirrors are official mirrors whose FQDNs have been CNAME'ed to a $CC.[archive,releases].ubuntu.com domain. This is why we refer to them as official country mirrors.

Stuart,

We're limited to one country mirror because it's not possible to load balance between two different Ubuntu archives mirrors because of limitations with the apt-get software unless the two mirrors are in perfect sync.

Also I believe, the plan is to, hopefully, auto-generate our CNAME DNS records of country mirrors using the ones specified in LP's DB by admins (with this patch). And it's not possible to have multiple CNAME records to a FQDN.

Revision history for this message
Michael Nelson (michael.nelson) wrote :

Hi Jonothan, thanks for yet another long-awaited feature :)

I'd really like to get Curtis to look at this one, although I've got a few comments.

The first question that came to my mind was, why is this another separate form (currently there is "Change details", "Review", and now "Set/Unset as country mirror".

I personally wonder whether the 'Review' form could accommodate this (perhaps renamed with something that implied official and/or country-mirror status?), so that both items could be presented and dealt with there on the one form. This would also alleviate the need for the changing menu item name ('Set/Unset'). But Curtis might have other ideas.

Also, when editing the mirror after setting it as a country mirror, the validation is great (for country/content-type), but afaik it shouldn't redirect back to the +index page, but instead +country-mirror form itself?

IRC snippet:

<noodles775> jpds: what's the reasoning behind not doing this as an admin-only field displayed on the normal edit form?
<jpds> noodles775: Do you mean the +review form?
<noodles775> jpds: no, I meant the +edit form (but i'm logged in as an admin, maybe that's not presented for mirror-admins...). Checking now.
<jpds> noodles775: I wanted to make it a little obvious by having the separate button for it.
<jpds> noodles775: And mirror registrants shouldn't be able to see the checkbox themselves.

review: Needs Information (ui)
Revision history for this message
Michael Nelson (michael.nelson) wrote :

> Also, when editing the mirror after setting it as a country mirror, the
> validation is great (for country/content-type), but afaik it shouldn't
> redirect back to the +index page, but instead +country-mirror form itself?

Sorry, that's very unclear (and incorrect). What I meant was, once you have set a mirror as a country mirror, when you then go back and change the details (on the +edit page), the validation there is great for country/content-type, but when that validation is triggered, the page should be redirected back to the +edit form, not back to the +index, as there could be other things that were updated by the user?

Revision history for this message
Jonathan Lange (jml) wrote :

OK, since it matches the existing documented domain language, I'm happy with the column name.

review: Approve (db)
Revision history for this message
Curtis Hovey (sinzui) wrote :
Download full text (22.2 KiB)

Hi Johnathan.

Thanks for undertaking this large effort.

I reviewed the code and the UI. There is a lot that needs fixing, but I
can help with some of it.

Starting with the UI.

I like your change to move country_dns_mirror to +edit, but per your original
concern, it is not obvious that the mirror admin uses this page to change the
state. We strive to move edit and view links into the page where the
information is presented so that it is clear to the user that he can change
what he is reading. This is a simple one-line addition to the template.

However, the addition of a new portlet makes the layout worse. This page
has known issues because it was one of the first to be changed for 3.0,
but it was not updated as 3.0 style evolved. The registration and status
information are not in their correct places: the registering slot and the
Mirror information portlet. If they were, the addition of the new field
would be easy and obvious. I tinkered with the template to fix the layout
and I have a diff that you can apply to your branch to get these changes.
In the fixed layout, the official country dns status appears higher on
the page and I think that is good.

    Suggested layout: http://people.canonical.com/~curtis/mirror.png
    Diff to revise layout: http://people.canonical.com/~curtis/mirror.diff

I do not like the info notifications. They make me worry that my other
changes were not accepted. Since the form did what I asked it to, there
is no need to take such an extraordinary measure. I think they should be
removed.

I pondered the addition of the star to the mirrors list. Your approach is
just like badges that are applied to bugs, so I think it is correct. I think
my misgivings are from my unfamiliarity with the star as an icon. I did
learn it was official when I placed my mouse over it. I think we can leave
it as is.

BTW. There is a problem with this merge proposal. It must be merged into
db-devel since the schema changes are incompatible with lpnet/edge. If you
had proposed the merge with db-devel you would have seen that your db
patch conflicts. You need to rename it and update it as suggested by Stuart
before this can be tested properly.

> === modified file 'lib/lp/registry/browser/distributionmirror.py'
> --- lib/lp/registry/browser/distributionmirror.py 2009-12-12 22:15:31 +0000
> +++ lib/lp/registry/browser/distributionmirror.py 2010-01-05 15:14:20 +0000

...

> @@ -275,10 +278,66 @@
> """See `LaunchpadFormView`."""
> return canonical_url(self.context)
>
> + def validate(self, data):
> + dns_mirror_choice = data.get('country_dns_mirror')
> +
> + # This mirror has not been marked as a country mirror.
> + if dns_mirror_choice == True:
> + # Safe-guard against multiple country mirrors for one country.
> + current_country_mirror = (
> + getUtility(IDistributionMirrorSet).getCountryMirrorForCountry(
> + self.context.country, self.context.content))
> +
> + # User has decided to mark it as one.
> + if current_country_mirror is None:
> + # There are none - mark this one as the one.
> + sel...

review: Needs Fixing (ui and code)
Revision history for this message
Jonathan Lange (jml) wrote :

Curtis & Michael, I can't believe that you _both_ spelled Jonathan's name incorrectly. :P

Revision history for this message
Michael Nelson (michael.nelson) wrote :

On Fri, Jan 8, 2010 at 5:59 AM, Jonathan Lange <email address hidden> wrote:
> Curtis & Michael, I can't believe that you _both_ spelled Jonathan's name incorrectly. :P

Sorry Jonathan... too many Aussie friends who go by "Jono" :/

> --
> https://code.edge.launchpad.net/~jpds/launchpad/fix_361650/+merge/16749
> You are reviewing the proposed merge of lp:~jpds/launchpad/fix_361650 into lp:launchpad/devel.
>

Revision history for this message
Graham Binns (gmb) wrote :

Can someone who knows the situation of this branch update its status? It'd be great if it was either WIP or Approved, just to get it out of the "reviews I can do" queue...

Revision history for this message
Curtis Hovey (sinzui) wrote :

I rejected this branch because the implementation and testing were unviable. Johnathan. Has landed the schema change and some prerequisite branches per my advice. His replacement branch to land this feature as a model change is in review.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'database/sampledata/current-dev.sql'
2--- database/sampledata/current-dev.sql 2009-12-14 13:49:03 +0000
3+++ database/sampledata/current-dev.sql 2010-01-05 15:14:20 +0000
4@@ -2219,6 +2219,9 @@
5 INSERT INTO distributionmirror (id, distribution, name, http_base_url, ftp_base_url, rsync_base_url, displayname, description, owner, speed, country, content, official_candidate, enabled, date_created, whiteboard, status, date_reviewed, reviewer) VALUES (8, 1, 'canonical-archive', 'http://archive.ubuntu.com/ubuntu/', NULL, NULL, NULL, NULL, 1, 70, 225, 1, true, true, '2006-10-16 18:31:43.434567', NULL, 30, NULL, NULL);
6 INSERT INTO distributionmirror (id, distribution, name, http_base_url, ftp_base_url, rsync_base_url, displayname, description, owner, speed, country, content, official_candidate, enabled, date_created, whiteboard, status, date_reviewed, reviewer) VALUES (9, 1, 'canonical-releases', 'http://releases.ubuntu.com/', NULL, NULL, NULL, NULL, 1, 70, 225, 2, true, true, '2006-10-16 18:31:43.434567', NULL, 30, NULL, NULL);
7 INSERT INTO distributionmirror (id, distribution, name, http_base_url, ftp_base_url, rsync_base_url, displayname, description, owner, speed, country, content, official_candidate, enabled, date_created, whiteboard, status, date_reviewed, reviewer) VALUES (10, 1, 'random-releases-mirror', 'http://releases.random.com/', NULL, NULL, NULL, NULL, 1, 70, 225, 2, true, true, '2006-10-16 18:31:43.434567', NULL, 10, NULL, NULL);
8+INSERT INTO distributionmirror (id, distribution, name, http_base_url, ftp_base_url, rsync_base_url, displayname, description, owner, speed, country, content, official_candidate, enabled, date_created, whiteboard, status, date_reviewed, reviewer) VALUES (11, 1, 'mirror.davis.antarctica.org-archive', 'http://mirror.davis.antarctica.org/ubuntu', NULL, NULL, 'Davis Station', NULL, 1, 70, 9, 1, true, true, '2010-01-03 00:07:44.412317', NULL, 30, NULL, NULL);
9+INSERT INTO distributionmirror (id, distribution, name, http_base_url, ftp_base_url, rsync_base_url, displayname, description, owner, speed, country, content, official_candidate, enabled, date_created, whiteboard, status, date_reviewed, reviewer) VALUES (12, 1, 'ubuntu.mirror.tudos.de-archive', 'http://ubuntu.mirror.tudos.de/ubuntu', NULL, 'rsync://ubuntu.mirror.tudos.de/ubuntu', 'Technische Universität Dresden', NULL, 12, 110, 82, 1, true, true, '2010-01-03 01:48:02.93419', NULL, 30, NULL, NULL);
10+INSERT INTO distributionmirror (id, distribution, name, http_base_url, ftp_base_url, rsync_base_url, displayname, description, owner, speed, country, content, official_candidate, enabled, date_created, whiteboard, status, date_reviewed, reviewer) VALUES (13, 1, 'mirror.mawson.antarctica.org-releases', 'http://mirror.mawson.antarctica.org/ubuntu-releases', NULL, NULL, 'Mawson Station', NULL, 1, 70, 9, 2, true, true, '2010-01-03 02:05:10.424767', NULL, 30, NULL, NULL);
11
12
13 ALTER TABLE distributionmirror ENABLE TRIGGER ALL;
14@@ -4857,6 +4860,9 @@
15
16 INSERT INTO mirrorproberecord (id, distribution_mirror, log_file, date_created) VALUES (1, 6, 46, '2006-05-24 17:11:59.37369');
17 INSERT INTO mirrorproberecord (id, distribution_mirror, log_file, date_created) VALUES (2, 7, 47, '2006-05-24 17:12:03.714206');
18+INSERT INTO mirrorproberecord (id, distribution_mirror, log_file, date_created) VALUES (3, 11, 47, '2010-01-03 00:20:30.412312');
19+INSERT INTO mirrorproberecord (id, distribution_mirror, log_file, date_created) VALUES (4, 12, 46, '2010-01-03 01:49:27.432231');
20+INSERT INTO mirrorproberecord (id, distribution_mirror, log_file, date_created) VALUES (5, 13, 47, '2010-01-03 02:07:17.434121');
21
22
23 ALTER TABLE mirrorproberecord ENABLE TRIGGER ALL;
24
25=== modified file 'database/sampledata/current.sql'
26--- database/sampledata/current.sql 2009-12-14 13:49:03 +0000
27+++ database/sampledata/current.sql 2010-01-05 15:14:20 +0000
28@@ -2201,6 +2201,9 @@
29 INSERT INTO distributionmirror (id, distribution, name, http_base_url, ftp_base_url, rsync_base_url, displayname, description, owner, speed, country, content, official_candidate, enabled, date_created, whiteboard, status, date_reviewed, reviewer) VALUES (8, 1, 'canonical-archive', 'http://archive.ubuntu.com/ubuntu/', NULL, NULL, NULL, NULL, 1, 70, 225, 1, true, true, '2006-10-16 18:31:43.434567', NULL, 30, NULL, NULL);
30 INSERT INTO distributionmirror (id, distribution, name, http_base_url, ftp_base_url, rsync_base_url, displayname, description, owner, speed, country, content, official_candidate, enabled, date_created, whiteboard, status, date_reviewed, reviewer) VALUES (9, 1, 'canonical-releases', 'http://releases.ubuntu.com/', NULL, NULL, NULL, NULL, 1, 70, 225, 2, true, true, '2006-10-16 18:31:43.434567', NULL, 30, NULL, NULL);
31 INSERT INTO distributionmirror (id, distribution, name, http_base_url, ftp_base_url, rsync_base_url, displayname, description, owner, speed, country, content, official_candidate, enabled, date_created, whiteboard, status, date_reviewed, reviewer) VALUES (10, 1, 'random-releases-mirror', 'http://releases.random.com/', NULL, NULL, NULL, NULL, 1, 70, 225, 2, true, true, '2006-10-16 18:31:43.434567', NULL, 10, NULL, NULL);
32+INSERT INTO distributionmirror (id, distribution, name, http_base_url, ftp_base_url, rsync_base_url, displayname, description, owner, speed, country, content, official_candidate, enabled, date_created, whiteboard, status, date_reviewed, reviewer) VALUES (11, 1, 'mirror.davis.antarctica.org-archive', 'http://mirror.davis.antarctica.org/ubuntu', NULL, NULL, 'Davis Station', NULL, 1, 70, 9, 1, true, true, '2010-01-03 00:07:44.412317', NULL, 30, NULL, NULL);
33+INSERT INTO distributionmirror (id, distribution, name, http_base_url, ftp_base_url, rsync_base_url, displayname, description, owner, speed, country, content, official_candidate, enabled, date_created, whiteboard, status, date_reviewed, reviewer) VALUES (12, 1, 'ubuntu.mirror.tudos.de-archive', 'http://ubuntu.mirror.tudos.de/ubuntu', NULL, 'rsync://ubuntu.mirror.tudos.de/ubuntu', 'Technische Universität Dresden', NULL, 12, 110, 82, 1, true, true, '2010-01-03 01:48:02.93419', NULL, 30, NULL, NULL);
34+INSERT INTO distributionmirror (id, distribution, name, http_base_url, ftp_base_url, rsync_base_url, displayname, description, owner, speed, country, content, official_candidate, enabled, date_created, whiteboard, status, date_reviewed, reviewer) VALUES (13, 1, 'mirror.mawson.antarctica.org-releases', 'http://mirror.mawson.antarctica.org/ubuntu-releases', NULL, NULL, 'Mawson Station', NULL, 1, 70, 9, 2, true, true, '2010-01-03 02:05:10.424767', NULL, 30, NULL, NULL);
35
36
37 ALTER TABLE distributionmirror ENABLE TRIGGER ALL;
38@@ -4776,6 +4779,9 @@
39
40 INSERT INTO mirrorproberecord (id, distribution_mirror, log_file, date_created) VALUES (1, 6, 46, '2006-05-24 17:11:59.37369');
41 INSERT INTO mirrorproberecord (id, distribution_mirror, log_file, date_created) VALUES (2, 7, 47, '2006-05-24 17:12:03.714206');
42+INSERT INTO mirrorproberecord (id, distribution_mirror, log_file, date_created) VALUES (3, 11, 47, '2010-01-03 00:20:30.412312');
43+INSERT INTO mirrorproberecord (id, distribution_mirror, log_file, date_created) VALUES (4, 12, 46, '2010-01-03 01:49:27.432231');
44+INSERT INTO mirrorproberecord (id, distribution_mirror, log_file, date_created) VALUES (5, 13, 47, '2010-01-03 02:07:17.434121');
45
46
47 ALTER TABLE mirrorproberecord ENABLE TRIGGER ALL;
48
49=== modified file 'database/schema/comments.sql'
50--- database/schema/comments.sql 2009-12-01 13:45:58 +0000
51+++ database/schema/comments.sql 2010-01-05 15:14:20 +0000
52@@ -1794,6 +1794,7 @@
53 COMMENT ON COLUMN DistributionMirror.whiteboard IS 'Notes on the current status of the mirror';
54 COMMENT ON COLUMN DistributionMirror.date_created IS 'The date and time the mirror was created.';
55 COMMENT ON COLUMN DistributionMirror.date_reviewed IS 'The date and time the mirror was reviewed.';
56+COMMENT ON COLUMN DistributionMirror.country_dns_mirror IS 'Is the mirror an country mirror in DNS?';
57
58 -- MirrorDistroArchSeries
59 COMMENT ON TABLE MirrorDistroArchSeries IS 'The mirror of the packages of a given Distro Arch Release.';
60
61=== added file 'database/schema/patch-2207-20-0.sql'
62--- database/schema/patch-2207-20-0.sql 1970-01-01 00:00:00 +0000
63+++ database/schema/patch-2207-20-0.sql 2010-01-05 15:14:20 +0000
64@@ -0,0 +1,19 @@
65+-- Copyright 2010 Canonical Ltd. This software is licensed under the
66+-- GNU Affero General Public License version 3 (see the file LICENSE).
67+
68+SET client_min_messages=ERROR;
69+
70+ALTER TABLE DistributionMirror
71+ ADD COLUMN country_dns_mirror boolean DEFAULT FALSE NOT NULL;
72+
73+--- Index for archive mirrors.
74+CREATE UNIQUE INDEX distributionmirror__archive__distribution__country__key
75+ON DistributionMirror(distribution, country, content)
76+WHERE country_dns_mirror IS TRUE AND content = 1;
77+
78+--- Index for releases mirrors.
79+CREATE UNIQUE INDEX distributionmirror__releases__distribution__country__key
80+ON DistributionMirror(distribution, country, content)
81+WHERE country_dns_mirror IS TRUE AND content = 2;
82+
83+INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 20, 0);
84
85=== modified file 'lib/lp/registry/browser/distributionmirror.py'
86--- lib/lp/registry/browser/distributionmirror.py 2009-12-12 22:15:31 +0000
87+++ lib/lp/registry/browser/distributionmirror.py 2010-01-05 15:14:20 +0000
88@@ -18,6 +18,7 @@
89 from datetime import datetime
90 import pytz
91
92+from zope.component import getUtility
93 from zope.lifecycleevent import ObjectCreatedEvent
94 from zope.event import notify
95 from zope.interface import implements
96@@ -32,7 +33,8 @@
97 from lp.registry.interfaces.distribution import (
98 IDistributionMirrorMenuMarker)
99 from lp.registry.interfaces.distributionmirror import (
100- IDistributionMirror)
101+ IDistributionMirror, IDistributionMirrorSet)
102+from canonical.launchpad.webapp.authorization import check_permission
103 from canonical.launchpad.webapp.batching import BatchNavigator
104 from canonical.launchpad.webapp.publisher import LaunchpadView
105 from canonical.launchpad.webapp import (
106@@ -259,7 +261,8 @@
107 schema = IDistributionMirror
108 field_names = ["name", "displayname", "description", "whiteboard",
109 "http_base_url", "ftp_base_url", "rsync_base_url", "speed",
110- "country", "content", "official_candidate"]
111+ "country", "content", "official_candidate",
112+ "country_dns_mirror"]
113 @property
114 def label(self):
115 """See `LaunchpadFormView`."""
116@@ -275,10 +278,66 @@
117 """See `LaunchpadFormView`."""
118 return canonical_url(self.context)
119
120+ def validate(self, data):
121+ dns_mirror_choice = data.get('country_dns_mirror')
122+
123+ # This mirror has not been marked as a country mirror.
124+ if dns_mirror_choice == True:
125+ # Safe-guard against multiple country mirrors for one country.
126+ current_country_mirror = (
127+ getUtility(IDistributionMirrorSet).getCountryMirrorForCountry(
128+ self.context.country, self.context.content))
129+
130+ # User has decided to mark it as one.
131+ if current_country_mirror is None:
132+ # There are none - mark this one as the one.
133+ self.request.response.addInfoNotification(
134+ "The mirror %s has been set as the country DNS mirror for "
135+ "%s." % (self.context.title, self.context.country.name))
136+ elif current_country_mirror == self.context:
137+ return # We are editing the country DNS mirror.
138+ else:
139+ # Make sure that there isn't already a mirror for this
140+ # country already.
141+ self.setFieldError("country_dns_mirror", "%s already "
142+ "has a country DNS mirror set, please unset the country "
143+ "DNS mirror status of %s before continuing." % (
144+ self.context.country.name,
145+ current_country_mirror.title))
146+ elif dns_mirror_choice == False and self.context.country_dns_mirror:
147+ self.request.response.addInfoNotification(
148+ "The mirror %s has been unset as the country DNS mirror for "
149+ "%s." % (self.context.title, self.context.country.name))
150+
151+ def setUpFields(self):
152+ super(DistributionMirrorEditView, self).setUpFields()
153+
154+ is_admin = check_permission('launchpad.Admin', self.context)
155+
156+ mirror_qualifies = (
157+ self.context.last_probe_record is not None
158+ and self.context.isOfficial()
159+ and self.context.http_base_url is not None)
160+
161+ # User must be an admin and the mirror must pass the above.
162+ if not is_admin or not mirror_qualifies:
163+ self.form_fields = self.form_fields.omit('country_dns_mirror')
164+
165 @action(_("Save"), name="save")
166 def action_save(self, action, data):
167- self.updateContextFromData(data)
168- self.next_url = canonical_url(self.context)
169+ # Ensure that we are not changing an archive mirror to a releases one
170+ # and that it's a country mirror at the same time.
171+ if data.get('content') != self.context.content:
172+ if self.context.country_dns_mirror:
173+ self.setFieldError("content", "Changing the mirror content "
174+ "type of country DNS mirrors is forbidden.")
175+ elif data.get('country') != self.context.country:
176+ if self.context.country_dns_mirror:
177+ self.setFieldError("country", "Changing the country of country "
178+ "DNS mirrors is forbidden.")
179+ else:
180+ self.updateContextFromData(data)
181+ self.next_url = canonical_url(self.context)
182
183
184 class DistributionMirrorReassignmentView(ObjectReassignmentView):
185
186=== modified file 'lib/lp/registry/browser/tests/distributionmirror-views.txt'
187--- lib/lp/registry/browser/tests/distributionmirror-views.txt 2009-12-14 15:22:53 +0000
188+++ lib/lp/registry/browser/tests/distributionmirror-views.txt 2010-01-05 15:14:20 +0000
189@@ -243,7 +243,7 @@
190 >>> view.field_names
191 ['name', 'displayname', 'description', 'whiteboard', 'http_base_url',
192 'ftp_base_url', 'rsync_base_url', 'speed', 'country', 'content',
193- 'official_candidate']
194+ 'official_candidate', 'country_dns_mirror']
195
196 >>> print mirror.ftp_base_url
197 None
198
199=== modified file 'lib/lp/registry/configure.zcml'
200--- lib/lp/registry/configure.zcml 2009-12-09 14:55:02 +0000
201+++ lib/lp/registry/configure.zcml 2010-01-05 15:14:20 +0000
202@@ -1611,7 +1611,7 @@
203 speed country content official_candidate owner" />
204 <require
205 permission="launchpad.Admin"
206- set_attributes="status reviewer date_reviewed"
207+ set_attributes="status reviewer date_reviewed country_dns_mirror"
208 attributes="destroySelf" />
209 </class>
210
211
212=== modified file 'lib/lp/registry/interfaces/distributionmirror.py'
213--- lib/lp/registry/interfaces/distributionmirror.py 2009-10-26 18:40:04 +0000
214+++ lib/lp/registry/interfaces/distributionmirror.py 2010-01-05 15:14:20 +0000
215@@ -330,8 +330,11 @@
216 'mirror of packages for this distribution.'),
217 vocabulary=MirrorContent)
218 official_candidate = Bool(
219- title=_('Apply to be an official mirror of this distribution'),
220+ title=_('Apply to be an official mirror of this distribution.'),
221 required=False, readonly=False, default=True)
222+ country_dns_mirror = Bool(
223+ title=_('Set this mirror as an country DNS mirror.'),
224+ required=False, readonly=False, default=False)
225 status = Choice(
226 title=_('Status'), required=True, readonly=False,
227 vocabulary=MirrorStatus)
228@@ -536,6 +539,11 @@
229 def getByRsyncUrl(url):
230 """Return the mirror with the given Rsync URL or None."""
231
232+ def getCountryMirrorForCountry(country, mirror_type):
233+ """Return the country mirror for a certain country for the specified
234+ mirror type.
235+ """
236+
237
238 class IMirrorDistroArchSeries(Interface):
239 """The mirror of the packages of a given Distro Arch Series"""
240
241=== modified file 'lib/lp/registry/model/distributionmirror.py'
242--- lib/lp/registry/model/distributionmirror.py 2009-10-26 18:40:04 +0000
243+++ lib/lp/registry/model/distributionmirror.py 2010-01-05 15:14:20 +0000
244@@ -94,6 +94,8 @@
245 notNull=True, enum=MirrorContent)
246 official_candidate = BoolCol(
247 notNull=True, default=False)
248+ country_dns_mirror = BoolCol(
249+ notNull=True, default=False)
250 status = EnumCol(
251 notNull=True, default=MirrorStatus.PENDING_REVIEW, enum=MirrorStatus)
252 date_created = UtcDateTimeCol(notNull=True, default=UTC_NOW)
253@@ -402,6 +404,26 @@
254 """See IDistributionMirrorSet"""
255 return DistributionMirror.get(mirror_id)
256
257+ def getCountryMirrorForCountry(self, country, mirror_type):
258+ """See IDistributionMirrorSet."""
259+ country_id = None
260+ if country is not None:
261+ country_id = country.id
262+
263+ base_query = AND(
264+ DistributionMirror.q.content == mirror_type,
265+ DistributionMirror.q.enabled == True,
266+ DistributionMirror.q.http_base_url != None,
267+ DistributionMirror.q.official_candidate == True,
268+ DistributionMirror.q.status == MirrorStatus.OFFICIAL,
269+ DistributionMirror.q.country_dns_mirror == True)
270+
271+ query = AND(DistributionMirror.q.countryID == country_id, base_query)
272+
273+ country_mirror = DistributionMirror.selectOne(query)
274+
275+ return country_mirror
276+
277 def getBestMirrorsForCountry(self, country, mirror_type):
278 """See IDistributionMirrorSet"""
279 # As per mvo's request we only return mirrors which have an
280
281=== modified file 'lib/lp/registry/stories/distributionmirror/xx-distribution-mirrors.txt'
282--- lib/lp/registry/stories/distributionmirror/xx-distribution-mirrors.txt 2009-12-10 23:32:13 +0000
283+++ lib/lp/registry/stories/distributionmirror/xx-distribution-mirrors.txt 2010-01-05 15:14:20 +0000
284@@ -41,12 +41,16 @@
285 Mirrors of Ubuntu Linux...
286 >>> print_mirrors_by_countries(browser.contents)
287 Antarctica:
288- [(u'Archive-mirror2', u'http', u'128 Kbps', u'Six hours behind')]
289+ [(u'Davis Station', u'', u'http', u'100 Mbps', u'Last update unknown'),
290+ (u'Archive-mirror2', u'', u'http', u'128 Kbps', u'Six hours behind')]
291 France:
292- [(u'Archive-404-mirror', u'http', u'512 Kbps', u'Last update unknown'),
293- (u'Archive-mirror', u'http', u'128 Kbps', u'Last update unknown')]
294+ [(u'Archive-404-mirror', u'', u'http', u'512 Kbps', u'Last update unknown'),
295+ (u'Archive-mirror', u'', u'http', u'128 Kbps', u'Last update unknown')]
296+ Germany:
297+ [(u'Technische Universit\xe4t Dresden', u'', u'http\nrsync', u'10 Gbps',
298+ u'Last update unknown')]
299 United Kingdom:
300- [(u'Canonical-archive', u'http', u'100 Mbps', u'Last update unknown')]
301+ [(u'Canonical-archive', u'', u'http', u'100 Mbps', u'Last update unknown')]
302 >>> find_tags_by_class(browser.contents, 'distromirrorstatusSIXHOURSBEHIND')
303 [<span class="distromirrorstatusSIXHOURSBEHIND">Six hours behind</span>]
304 >>> find_tags_by_class(browser.contents, 'distromirrorstatusUNKNOWN')[0]
305@@ -59,13 +63,160 @@
306 >>> browser.url
307 'http://launchpad.dev/ubuntu/+cdmirrors'
308 >>> print_mirrors_by_countries(browser.contents)
309+ Antarctica: [(u'Mawson Station', u'', u'http', u'100 Mbps')]
310 France:
311- [(u'Releases-mirror', u'http', u'2 Mbps'),
312- (u'Unreachable-mirror', u'http', u'512 Kbps')]
313+ [(u'Releases-mirror', u'', u'http', u'2 Mbps'),
314+ (u'Unreachable-mirror', u'', u'http', u'512 Kbps')]
315 Germany:
316- [(u'Releases-mirror2', u'http', u'2 Mbps')]
317+ [(u'Releases-mirror2', u'', u'http', u'2 Mbps')]
318 United Kingdom:
319- [(u'Canonical-releases', u'http', u'100 Mbps')]
320+ [(u'Canonical-releases', u'', u'http', u'100 Mbps')]
321+
322+
323+=== Country DNS mirrors ===
324+
325+Country DNS mirrors are official mirrors whose FQDN have been CNAME'd for
326+domain records such as gb.archive.ubuntu.com. They are highly reliable and set
327+during installation, depending on which country the user specified they were
328+in.
329+
330+Mirror listing administrators are the ones with the authority to set mirrors as
331+country mirrors.
332+
333+ >>> browser = setupBrowser(auth='Basic karl@canonical.com:test')
334+ >>> browser.open('https://launchpad.dev/ubuntu/+mirror/archive-mirror2')
335+ >>> browser.getLink("Change details").click()
336+ >>> print browser.title
337+ Edit mirror Archive-mirror2...
338+ >>> browser.getControl(name='field.country_dns_mirror').value = True
339+ >>> browser.getControl('Save').click()
340+ >>> print find_tags_by_class(browser.contents, 'informational message')[0]
341+ <div class="informational message">The mirror Archive-mirror2 has been set as
342+ the country DNS mirror for Antarctica.</div>
343+ >>> print find_tag_by_id(browser.contents, 'maincontent').renderContents()
344+ <BLANKLINE>
345+ ...Country DNS mirror...
346+ <BLANKLINE>
347+ ...This mirror is the country DNS mirror for <span>Antarctica</span>...
348+
349+And also to unset mirrors as country mirrors:
350+
351+ >>> browser = setupBrowser(auth='Basic karl@canonical.com:test')
352+ >>> browser.open('https://launchpad.dev/ubuntu/+mirror/archive-mirror2')
353+ >>> browser.getLink("Change details").click()
354+ >>> print browser.title
355+ Edit mirror Archive-mirror2...
356+ >>> browser.getControl(name='field.country_dns_mirror').value = False
357+ >>> browser.getControl('Save').click()
358+ >>> print find_tags_by_class(browser.contents, 'informational message')[0]
359+ <div class="informational message">The mirror Archive-mirror2 has been
360+ unset as the country DNS mirror for Antarctica.</div>
361+
362+Unofficial/pending-review mirrors may not be set as country mirrors:
363+
364+ >>> browser = setupBrowser(auth='Basic karl@canonical.com:test')
365+ >>> browser.open('https://launchpad.dev/ubuntu/+mirror/invalid-mirror/+edit')
366+ >>> browser.getControl(
367+ ... name='field.country_dns_mirror').value = True
368+ Traceback (most recent call last):
369+ ...
370+ LookupError: name 'field.country_dns_mirror'
371+
372+Mirror registrants may not make changes to country DNS mirrors settings:
373+
374+ >>> browser_mirror_registrant = setupBrowser(
375+ ... auth='Basic test@canonical.com:test')
376+ >>> browser_mirror_registrant.open(
377+ ... 'https://launchpad.dev/ubuntu/+mirror/ubuntu.mirror.tudos.de-archive/+edit')
378+ >>> browser_mirror_registrant.getControl(
379+ ... name='field.country_dns_mirror').value = True
380+ Traceback (most recent call last):
381+ ...
382+ LookupError: name 'field.country_dns_mirror'
383+
384+A country may not have more than one country content-type mirror, for example,
385+archive mirrors:
386+
387+ >>> browser = setupBrowser(auth='Basic karl@canonical.com:test')
388+ >>> browser.open(
389+ ... 'https://launchpad.dev/ubuntu/+mirror/mirror.davis.antarctica.org-archive')
390+ >>> browser.getLink("Change details").click()
391+ >>> print browser.title
392+ Edit mirror Davis Station...
393+ >>> browser.getControl(name='field.country_dns_mirror').value = True
394+ >>> browser.getControl('Save').click()
395+ >>> print find_tags_by_class(browser.contents, 'informational message')[0]
396+ <div class="informational message">The mirror Davis Station has been set
397+ as the country DNS mirror for Antarctica.</div>
398+ >>> browser.open('https://launchpad.dev/ubuntu/+mirror/archive-mirror2')
399+ >>> browser.getLink("Change details").click()
400+ >>> print browser.title
401+ Edit mirror Archive-mirror2...
402+ >>> browser.getControl(name='field.country_dns_mirror').value = True
403+ >>> browser.getControl('Save').click()
404+ >>> print find_tags_by_class(browser.contents, 'message')
405+ [<p class="error message">There is 1 error.</p>,
406+ <div class="message">Antarctica already has a country DNS mirror set, please
407+ unset the country DNS mirror status of Davis Station before
408+ continuing.</div>]
409+
410+We can however set a country archive mirror for another country in the
411+mean-time, just fine:
412+
413+ >>> browser = setupBrowser(auth='Basic karl@canonical.com:test')
414+ >>> browser.open(
415+ ... 'https://launchpad.dev/ubuntu/+mirror/ubuntu.mirror.tudos.de-archive')
416+ >>> browser.getLink("Change details").click()
417+ >>> print browser.title
418+ Edit mirror Technische Universität Dresden...
419+ >>> browser.getControl(name='field.country_dns_mirror').value = True
420+ >>> browser.getControl('Save').click()
421+ >>> print find_tags_by_class(browser.contents, 'informational message')[0]
422+ <div class="informational message">The mirror Technische Universität
423+ Dresden has been set as the country DNS mirror for Germany.</div>
424+
425+Setting releases mirrors as country releases mirrors should not conflict with
426+country archive mirrors settings:
427+
428+ >>> browser = setupBrowser(auth='Basic karl@canonical.com:test')
429+ >>> browser.open(
430+ ... 'https://launchpad.dev/ubuntu/+mirror/mirror.mawson.antarctica.org-releases')
431+ >>> browser.getLink("Change details").click()
432+ >>> print browser.title
433+ Edit mirror Mawson Station...
434+ >>> browser.getControl(name='field.country_dns_mirror').value = True
435+ >>> browser.getControl('Save').click()
436+ >>> print find_tags_by_class(browser.contents, 'informational message')[0]
437+ <div class="informational message">The mirror Mawson Station has been set
438+ as the country DNS mirror for Antarctica.</div>
439+
440+Once a mirror has been set as a country mirror, it's content type or country
441+may not be changed:
442+
443+ >>> browser = setupBrowser(auth='Basic karl@canonical.com:test')
444+ >>> browser.open(
445+ ... 'https://launchpad.dev/ubuntu/+mirror/mirror.mawson.antarctica.org-releases')
446+ >>> browser.getLink("Change details").click()
447+ >>> print browser.title
448+ Edit mirror Mawson Station...
449+ >>> browser.getControl(name='field.content').value = ["ARCHIVE"]
450+ >>> browser.getControl('Save').click()
451+ >>> print find_tags_by_class(browser.contents, 'message')
452+ [<p class="error message">There is 1 error.</p>,
453+ <div class="message">Changing the mirror content type of country DNS
454+ mirrors is forbidden.</div>]
455+ >>> browser.open(
456+ ... 'https://launchpad.dev/ubuntu/+mirror/mirror.mawson.antarctica.org-releases')
457+ >>> browser.getLink("Change details").click()
458+ >>> print browser.title
459+ Edit mirror Mawson Station...
460+ >>> browser.getControl(name='field.country').value = ['82'] # Germany.
461+ >>> browser.getControl('Save').click()
462+ >>> print find_tags_by_class(browser.contents, 'message')
463+ [<p class="error message">There is 1 error.</p>,
464+ <div class="message">Changing the country of country DNS mirrors is
465+ forbidden.</div>]
466+
467
468 === Disabled mirrors ===
469
470@@ -105,7 +256,7 @@
471 'http://launchpad.dev/ubuntu/+unofficialmirrors'
472
473 >>> print_mirrors_by_countries(browser.contents)
474- France: [(u'Invalid-mirror', u'http', u'2 Mbps', u'Last update unknown')]
475+ France: [(u'Invalid-mirror', u'', u'http', u'2 Mbps', u'Last update unknown')]
476
477 == Pending-review mirrors ==
478
479@@ -133,8 +284,8 @@
480 >>> browser.open('http://launchpad.dev/ubuntu/+pendingreviewmirrors')
481 >>> print_mirrors_by_countries(browser.contents)
482 Afghanistan:
483- [(u'Kabul LUG mirror', u'ftp', u'10 Gbps',
484+ [(u'Kabul LUG mirror', u'', u'ftp', u'10 Gbps',
485 u'Archive')]
486 United Kingdom:
487- [(u'Random-releases-mirror', u'http', u'100 Mbps',
488+ [(u'Random-releases-mirror', u'', u'http', u'100 Mbps',
489 u'CD Image')]
490
491=== modified file 'lib/lp/registry/templates/distributionmirror-index.pt'
492--- lib/lp/registry/templates/distributionmirror-index.pt 2009-12-08 15:51:34 +0000
493+++ lib/lp/registry/templates/distributionmirror-index.pt 2010-01-05 15:14:20 +0000
494@@ -76,6 +76,15 @@
495 </div>
496 </div>
497 </div>
498+ <div tal:condition="context/country_dns_mirror">
499+ <div class="portlet">
500+ <h2>Country DNS mirror</h2>
501+ <p>
502+ This mirror is the country DNS mirror for <span
503+ tal:content="context/country/name" />.
504+ </p>
505+ </div>
506+ </div>
507
508 <div class="portlet"
509 id="last-probe"
510
511=== modified file 'lib/lp/registry/templates/distributionmirror-macros.pt'
512--- lib/lp/registry/templates/distributionmirror-macros.pt 2009-12-09 19:41:23 +0000
513+++ lib/lp/registry/templates/distributionmirror-macros.pt 2010-01-05 15:14:20 +0000
514@@ -19,6 +19,7 @@
515 <tr class="highlighted">
516 <th colspan="2" style="text-align: left"
517 tal:content="country_and_mirrors/country" />
518+ <th align="right" />
519 <th style="text-align: left"
520 tal:content="country_and_mirrors/throughput"/>
521 <th style="text-align: left" tal:condition="show_mirror_type">
522@@ -36,6 +37,10 @@
523 <a tal:attributes="href mirror/fmt:url"
524 tal:content="mirror/title">Mirror Name</a>
525 </td>
526+ <td align="right">
527+ <img tal:condition="mirror/country_dns_mirror" src="/@@/favourite-yes"
528+ title="Country DNS mirror" />
529+ </td>
530 <td>
531 <a tal:condition="mirror/http_base_url"
532 tal:attributes="href mirror/http_base_url">http</a>
533@@ -65,10 +70,11 @@
534
535 </tal:country_and_mirrors>
536 <tr class="highlighted">
537- <th colspan="5" style="text-align: left; font-weight: bold;">Total</th>
538+ <th colspan="6" style="text-align: left; font-weight: bold;">Total</th>
539 </tr>
540 <tr>
541 <td colspan="2" />
542+ <td />
543 <td style="text-align: left; font-weight: bold;"
544 tal:content="total_throughput" />
545 <td tal:condition="show_mirror_type"></td>