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

[Jonathan Davies]

[Jonathan Davies (jpds)]

= 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.

[Jonathan Lange (jml)]

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)

[Stuart Bishop (stub)]

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)

[Stuart Bishop (stub)]

> 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.

[Jonathan Davies (jpds)]

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.

[Michael Nelson (michael.nelson)]

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)

[Michael Nelson (michael.nelson)]

> 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?

[Jonathan Lange (jml)]

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

review: Approve (db)

[Curtis Hovey (sinzui)]

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)

[Jonathan Lange (jml)]

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

[Michael Nelson (michael.nelson)]

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.
>

[Graham Binns (gmb)]

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...

[Curtis Hovey (sinzui)]

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.

=== modified file 'database/sampledata/current-dev.sql'
--- database/sampledata/current-dev.sql	2009-12-14 13:49:03 +0000
+++ database/sampledata/current-dev.sql	2010-01-05 15:14:20 +0000
@@ -2219,6 +2219,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 (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);
 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);
 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);
+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);
+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);
+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);
 
 
 ALTER TABLE distributionmirror ENABLE TRIGGER ALL;
@@ -4857,6 +4860,9 @@
 
 INSERT INTO mirrorproberecord (id, distribution_mirror, log_file, date_created) VALUES (1, 6, 46, '2006-05-24 17:11:59.37369');
 INSERT INTO mirrorproberecord (id, distribution_mirror, log_file, date_created) VALUES (2, 7, 47, '2006-05-24 17:12:03.714206');
+INSERT INTO mirrorproberecord (id, distribution_mirror, log_file, date_created) VALUES (3, 11, 47, '2010-01-03 00:20:30.412312');
+INSERT INTO mirrorproberecord (id, distribution_mirror, log_file, date_created) VALUES (4, 12, 46, '2010-01-03 01:49:27.432231');
+INSERT INTO mirrorproberecord (id, distribution_mirror, log_file, date_created) VALUES (5, 13, 47, '2010-01-03 02:07:17.434121');
 
 
 ALTER TABLE mirrorproberecord ENABLE TRIGGER ALL;
 
=== modified file 'database/sampledata/current.sql'
--- database/sampledata/current.sql	2009-12-14 13:49:03 +0000
+++ database/sampledata/current.sql	2010-01-05 15:14:20 +0000
@@ -2201,6 +2201,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 (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);
 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);
 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);
+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);
+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);
+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);
 
 
 ALTER TABLE distributionmirror ENABLE TRIGGER ALL;
@@ -4776,6 +4779,9 @@
 
 INSERT INTO mirrorproberecord (id, distribution_mirror, log_file, date_created) VALUES (1, 6, 46, '2006-05-24 17:11:59.37369');
 INSERT INTO mirrorproberecord (id, distribution_mirror, log_file, date_created) VALUES (2, 7, 47, '2006-05-24 17:12:03.714206');
+INSERT INTO mirrorproberecord (id, distribution_mirror, log_file, date_created) VALUES (3, 11, 47, '2010-01-03 00:20:30.412312');
+INSERT INTO mirrorproberecord (id, distribution_mirror, log_file, date_created) VALUES (4, 12, 46, '2010-01-03 01:49:27.432231');
+INSERT INTO mirrorproberecord (id, distribution_mirror, log_file, date_created) VALUES (5, 13, 47, '2010-01-03 02:07:17.434121');
 
 
 ALTER TABLE mirrorproberecord ENABLE TRIGGER ALL;
 
=== modified file 'database/schema/comments.sql'
--- database/schema/comments.sql	2009-12-01 13:45:58 +0000
+++ database/schema/comments.sql	2010-01-05 15:14:20 +0000
@@ -1794,6 +1794,7 @@
 COMMENT ON COLUMN DistributionMirror.whiteboard IS 'Notes on the current status of the mirror';
 COMMENT ON COLUMN DistributionMirror.date_created IS 'The date and time the mirror was created.';
 COMMENT ON COLUMN DistributionMirror.date_reviewed IS 'The date and time the mirror was reviewed.';
+COMMENT ON COLUMN DistributionMirror.country_dns_mirror IS 'Is the mirror an country mirror in DNS?';
 
 -- MirrorDistroArchSeries
 COMMENT ON TABLE MirrorDistroArchSeries IS 'The mirror of the packages of a given Distro Arch Release.';
 
=== added file 'database/schema/patch-2207-20-0.sql'
--- database/schema/patch-2207-20-0.sql	1970-01-01 00:00:00 +0000
+++ database/schema/patch-2207-20-0.sql	2010-01-05 15:14:20 +0000
@@ -0,0 +1,19 @@
+-- Copyright 2010 Canonical Ltd.  This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+
+SET client_min_messages=ERROR;
+
+ALTER TABLE DistributionMirror
+    ADD COLUMN country_dns_mirror boolean DEFAULT FALSE NOT NULL;
+
+--- Index for archive mirrors.
+CREATE UNIQUE INDEX distributionmirror__archive__distribution__country__key
+ON DistributionMirror(distribution, country, content)
+WHERE country_dns_mirror IS TRUE AND content = 1;
+
+--- Index for releases mirrors.
+CREATE UNIQUE INDEX distributionmirror__releases__distribution__country__key
+ON DistributionMirror(distribution, country, content)
+WHERE country_dns_mirror IS TRUE AND content = 2;
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 20, 0);
 
=== 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
@@ -18,6 +18,7 @@
 from datetime import datetime
 import pytz
 
+from zope.component import getUtility
 from zope.lifecycleevent import ObjectCreatedEvent
 from zope.event import notify
 from zope.interface import implements
@@ -32,7 +33,8 @@
 from lp.registry.interfaces.distribution import (
     IDistributionMirrorMenuMarker)
 from lp.registry.interfaces.distributionmirror import (
-    IDistributionMirror)
+    IDistributionMirror, IDistributionMirrorSet)
+from canonical.launchpad.webapp.authorization import check_permission
 from canonical.launchpad.webapp.batching import BatchNavigator
 from canonical.launchpad.webapp.publisher import LaunchpadView
 from canonical.launchpad.webapp import (
@@ -259,7 +261,8 @@
     schema = IDistributionMirror
     field_names = ["name", "displayname", "description", "whiteboard",
                    "http_base_url", "ftp_base_url", "rsync_base_url", "speed",
-                   "country", "content", "official_candidate"]
+                   "country", "content", "official_candidate",
+                   "country_dns_mirror"]
     @property
     def label(self):
         """See `LaunchpadFormView`."""
@@ -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.
+                self.request.response.addInfoNotification(
+                    "The mirror %s has been set as the country DNS mirror for "
+                    "%s." % (self.context.title, self.context.country.name))
+            elif current_country_mirror == self.context:
+                return # We are editing the country DNS mirror.
+            else:
+                 # Make sure that there isn't already a mirror for this
+                 # country already.
+                 self.setFieldError("country_dns_mirror", "%s already "
+                    "has a country DNS mirror set, please unset the country "
+                    "DNS mirror status of %s before continuing." % (
+                    self.context.country.name,
+                    current_country_mirror.title))
+        elif dns_mirror_choice == False and self.context.country_dns_mirror:
+            self.request.response.addInfoNotification(
+                "The mirror %s has been unset as the country DNS mirror for "
+                "%s." % (self.context.title, self.context.country.name))
+
+    def setUpFields(self):
+        super(DistributionMirrorEditView, self).setUpFields()
+        
+        is_admin = check_permission('launchpad.Admin', self.context)
+        
+        mirror_qualifies = (
+            self.context.last_probe_record is not None
+            and self.context.isOfficial()
+            and self.context.http_base_url is not None)
+
+        # User must be an admin and the mirror must pass the above.
+        if not is_admin or not mirror_qualifies:
+            self.form_fields = self.form_fields.omit('country_dns_mirror')
+
     @action(_("Save"), name="save")
     def action_save(self, action, data):
-        self.updateContextFromData(data)
-        self.next_url = canonical_url(self.context)
+        # Ensure that we are not changing an archive mirror to a releases one
+        # and that it's a country mirror at the same time.
+        if data.get('content') != self.context.content:
+            if self.context.country_dns_mirror:
+                self.setFieldError("content", "Changing the mirror content "
+                    "type of country DNS mirrors is forbidden.")
+        elif data.get('country') != self.context.country:
+            if self.context.country_dns_mirror:
+                self.setFieldError("country", "Changing the country of country "
+                    "DNS mirrors is forbidden.")
+        else:
+            self.updateContextFromData(data)
+            self.next_url = canonical_url(self.context)
 
 
 class DistributionMirrorReassignmentView(ObjectReassignmentView):
 
=== modified file 'lib/lp/registry/browser/tests/distributionmirror-views.txt'
--- lib/lp/registry/browser/tests/distributionmirror-views.txt	2009-12-14 15:22:53 +0000
+++ lib/lp/registry/browser/tests/distributionmirror-views.txt	2010-01-05 15:14:20 +0000
@@ -243,7 +243,7 @@
     >>> view.field_names
     ['name', 'displayname', 'description', 'whiteboard', 'http_base_url',
      'ftp_base_url', 'rsync_base_url', 'speed', 'country', 'content',
-     'official_candidate']
+     'official_candidate', 'country_dns_mirror']
 
     >>> print mirror.ftp_base_url
     None
 
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2009-12-09 14:55:02 +0000
+++ lib/lp/registry/configure.zcml	2010-01-05 15:14:20 +0000
@@ -1611,7 +1611,7 @@
                             speed country content official_candidate owner" />
         <require
             permission="launchpad.Admin"
-            set_attributes="status reviewer date_reviewed"
+            set_attributes="status reviewer date_reviewed country_dns_mirror"
             attributes="destroySelf" />
     </class>
 
 
=== modified file 'lib/lp/registry/interfaces/distributionmirror.py'
--- lib/lp/registry/interfaces/distributionmirror.py	2009-10-26 18:40:04 +0000
+++ lib/lp/registry/interfaces/distributionmirror.py	2010-01-05 15:14:20 +0000
@@ -330,8 +330,11 @@
             'mirror of packages for this distribution.'),
         vocabulary=MirrorContent)
     official_candidate = Bool(
-        title=_('Apply to be an official mirror of this distribution'),
+        title=_('Apply to be an official mirror of this distribution.'),
         required=False, readonly=False, default=True)
+    country_dns_mirror = Bool(
+        title=_('Set this mirror as an country DNS mirror.'),
+        required=False, readonly=False, default=False)
     status = Choice(
         title=_('Status'), required=True, readonly=False,
         vocabulary=MirrorStatus)
@@ -536,6 +539,11 @@
     def getByRsyncUrl(url):
         """Return the mirror with the given Rsync URL or None."""
 
+    def getCountryMirrorForCountry(country, mirror_type):
+        """Return the country mirror for a certain country for the specified
+        mirror type.
+        """
+
 
 class IMirrorDistroArchSeries(Interface):
     """The mirror of the packages of a given Distro Arch Series"""
 
=== modified file 'lib/lp/registry/model/distributionmirror.py'
--- lib/lp/registry/model/distributionmirror.py	2009-10-26 18:40:04 +0000
+++ lib/lp/registry/model/distributionmirror.py	2010-01-05 15:14:20 +0000
@@ -94,6 +94,8 @@
         notNull=True, enum=MirrorContent)
     official_candidate = BoolCol(
         notNull=True, default=False)
+    country_dns_mirror = BoolCol(
+        notNull=True, default=False)
     status = EnumCol(
         notNull=True, default=MirrorStatus.PENDING_REVIEW, enum=MirrorStatus)
     date_created = UtcDateTimeCol(notNull=True, default=UTC_NOW)
@@ -402,6 +404,26 @@
         """See IDistributionMirrorSet"""
         return DistributionMirror.get(mirror_id)
 
+    def getCountryMirrorForCountry(self, country, mirror_type):
+        """See IDistributionMirrorSet."""
+        country_id = None
+        if country is not None:
+            country_id = country.id
+
+        base_query = AND(
+            DistributionMirror.q.content == mirror_type,
+            DistributionMirror.q.enabled == True,
+            DistributionMirror.q.http_base_url != None,
+            DistributionMirror.q.official_candidate == True,
+            DistributionMirror.q.status == MirrorStatus.OFFICIAL,
+            DistributionMirror.q.country_dns_mirror == True)
+
+        query = AND(DistributionMirror.q.countryID == country_id, base_query)
+
+        country_mirror = DistributionMirror.selectOne(query)
+
+        return country_mirror
+
     def getBestMirrorsForCountry(self, country, mirror_type):
         """See IDistributionMirrorSet"""
         # As per mvo's request we only return mirrors which have an
 
=== modified file 'lib/lp/registry/stories/distributionmirror/xx-distribution-mirrors.txt'
--- lib/lp/registry/stories/distributionmirror/xx-distribution-mirrors.txt	2009-12-10 23:32:13 +0000
+++ lib/lp/registry/stories/distributionmirror/xx-distribution-mirrors.txt	2010-01-05 15:14:20 +0000
@@ -41,12 +41,16 @@
     Mirrors of Ubuntu Linux...
     >>> print_mirrors_by_countries(browser.contents)
     Antarctica:
-      [(u'Archive-mirror2',    u'http', u'128 Kbps', u'Six hours behind')]
+      [(u'Davis Station', u'', u'http', u'100 Mbps', u'Last update unknown'),
+       (u'Archive-mirror2', u'', u'http', u'128 Kbps', u'Six hours behind')]
     France:
-      [(u'Archive-404-mirror', u'http', u'512 Kbps', u'Last update unknown'),
-       (u'Archive-mirror',     u'http', u'128 Kbps', u'Last update unknown')]
+      [(u'Archive-404-mirror', u'', u'http', u'512 Kbps', u'Last update unknown'),
+       (u'Archive-mirror', u'', u'http', u'128 Kbps', u'Last update unknown')]
+    Germany:
+      [(u'Technische Universit\xe4t Dresden', u'', u'http\nrsync', u'10 Gbps',
+        u'Last update unknown')]
     United Kingdom:
-      [(u'Canonical-archive',  u'http', u'100 Mbps', u'Last update unknown')]
+      [(u'Canonical-archive', u'', u'http', u'100 Mbps', u'Last update unknown')]
     >>> find_tags_by_class(browser.contents, 'distromirrorstatusSIXHOURSBEHIND')
     [<span class="distromirrorstatusSIXHOURSBEHIND">Six hours behind</span>]
     >>> find_tags_by_class(browser.contents, 'distromirrorstatusUNKNOWN')[0]
@@ -59,13 +63,160 @@
     >>> browser.url
     'http://launchpad.dev/ubuntu/+cdmirrors'
     >>> print_mirrors_by_countries(browser.contents)
+    Antarctica: [(u'Mawson Station', u'', u'http', u'100 Mbps')]
     France:
-      [(u'Releases-mirror',    u'http', u'2 Mbps'),
-       (u'Unreachable-mirror', u'http', u'512 Kbps')]
+      [(u'Releases-mirror', u'', u'http', u'2 Mbps'),
+       (u'Unreachable-mirror', u'', u'http', u'512 Kbps')]
     Germany:
-      [(u'Releases-mirror2',   u'http', u'2 Mbps')]
+      [(u'Releases-mirror2', u'', u'http', u'2 Mbps')]
     United Kingdom:
-      [(u'Canonical-releases', u'http', u'100 Mbps')]
+      [(u'Canonical-releases', u'', u'http', u'100 Mbps')]
+
+
+=== Country DNS mirrors ===
+
+Country DNS mirrors are official mirrors whose FQDN have been CNAME'd for
+domain records such as gb.archive.ubuntu.com. They are highly reliable and set
+during installation, depending on which country the user specified they were
+in.
+
+Mirror listing administrators are the ones with the authority to set mirrors as
+country mirrors.
+
+    >>> browser = setupBrowser(auth='Basic [e-mail]:test')
+    >>> browser.open('https://launchpad.dev/ubuntu/+mirror/archive-mirror2')
+    >>> browser.getLink("Change details").click()
+    >>> print browser.title
+    Edit mirror Archive-mirror2...
+    >>> browser.getControl(name='field.country_dns_mirror').value = True
+    >>> browser.getControl('Save').click()
+    >>> print find_tags_by_class(browser.contents, 'informational message')[0]
+    <div class="informational message">The mirror Archive-mirror2 has been set as
+     the country DNS mirror for Antarctica.</div>
+    >>> print find_tag_by_id(browser.contents, 'maincontent').renderContents()
+    <BLANKLINE>
+    ...Country DNS mirror...
+    <BLANKLINE>
+    ...This mirror is the country DNS mirror for <span>Antarctica</span>...
+
+And also to unset mirrors as country mirrors:
+
+    >>> browser = setupBrowser(auth='Basic [e-mail]:test')
+    >>> browser.open('https://launchpad.dev/ubuntu/+mirror/archive-mirror2')
+    >>> browser.getLink("Change details").click()
+    >>> print browser.title
+    Edit mirror Archive-mirror2...
+    >>> browser.getControl(name='field.country_dns_mirror').value = False
+    >>> browser.getControl('Save').click()
+    >>> print find_tags_by_class(browser.contents, 'informational message')[0]
+    <div class="informational message">The mirror Archive-mirror2 has been
+     unset as the country DNS mirror for Antarctica.</div>
+
+Unofficial/pending-review mirrors may not be set as country mirrors:
+
+    >>> browser = setupBrowser(auth='Basic [e-mail]:test')
+    >>> browser.open('https://launchpad.dev/ubuntu/+mirror/invalid-mirror/+edit')
+    >>> browser.getControl(
+    ...     name='field.country_dns_mirror').value = True
+    Traceback (most recent call last):
+    ...
+    LookupError: name 'field.country_dns_mirror'
+
+Mirror registrants may not make changes to country DNS mirrors settings:
+
+    >>> browser_mirror_registrant = setupBrowser(
+    ...     auth='Basic [e-mail]:test')
+    >>> browser_mirror_registrant.open(
+    ...     'https://launchpad.dev/ubuntu/+mirror/ubuntu.mirror.tudos.de-archive/+edit')
+    >>> browser_mirror_registrant.getControl(
+    ...     name='field.country_dns_mirror').value = True
+    Traceback (most recent call last):
+    ...
+    LookupError: name 'field.country_dns_mirror'
+
+A country may not have more than one country content-type mirror, for example,
+archive mirrors:
+
+    >>> browser = setupBrowser(auth='Basic [e-mail]:test')
+    >>> browser.open(
+    ...     'https://launchpad.dev/ubuntu/+mirror/mirror.davis.antarctica.org-archive')
+    >>> browser.getLink("Change details").click()
+    >>> print browser.title
+    Edit mirror Davis Station...
+    >>> browser.getControl(name='field.country_dns_mirror').value = True
+    >>> browser.getControl('Save').click()
+    >>> print find_tags_by_class(browser.contents, 'informational message')[0]
+    <div class="informational message">The mirror Davis Station has been set 
+     as the country DNS mirror for Antarctica.</div>
+    >>> browser.open('https://launchpad.dev/ubuntu/+mirror/archive-mirror2')
+    >>> browser.getLink("Change details").click()
+    >>> print browser.title
+    Edit mirror Archive-mirror2...
+    >>> browser.getControl(name='field.country_dns_mirror').value = True
+    >>> browser.getControl('Save').click()
+    >>> print find_tags_by_class(browser.contents, 'message')
+    [<p class="error message">There is 1 error.</p>,
+     <div class="message">Antarctica already has a country DNS mirror set, please
+     unset the country DNS mirror status of Davis Station before
+     continuing.</div>]
+
+We can however set a country archive mirror for another country in the
+mean-time, just fine:
+
+    >>> browser = setupBrowser(auth='Basic [e-mail]:test')
+    >>> browser.open(
+    ...     'https://launchpad.dev/ubuntu/+mirror/ubuntu.mirror.tudos.de-archive')
+    >>> browser.getLink("Change details").click()
+    >>> print browser.title
+    Edit mirror Technische Universität Dresden...
+    >>> browser.getControl(name='field.country_dns_mirror').value = True
+    >>> browser.getControl('Save').click()
+    >>> print find_tags_by_class(browser.contents, 'informational message')[0]
+    <div class="informational message">The mirror Technische Universität
+     Dresden has been set as the country DNS mirror for Germany.</div>
+
+Setting releases mirrors as country releases mirrors should not conflict with
+country archive mirrors settings:
+
+    >>> browser = setupBrowser(auth='Basic [e-mail]:test')
+    >>> browser.open(
+    ...     'https://launchpad.dev/ubuntu/+mirror/mirror.mawson.antarctica.org-releases')
+    >>> browser.getLink("Change details").click()
+    >>> print browser.title
+    Edit mirror Mawson Station...
+    >>> browser.getControl(name='field.country_dns_mirror').value = True
+    >>> browser.getControl('Save').click()
+    >>> print find_tags_by_class(browser.contents, 'informational message')[0]
+    <div class="informational message">The mirror Mawson Station has been set
+     as the country DNS mirror for Antarctica.</div>
+
+Once a mirror has been set as a country mirror, it's content type or country
+may not be changed:
+
+    >>> browser = setupBrowser(auth='Basic [e-mail]:test')
+    >>> browser.open(
+    ...     'https://launchpad.dev/ubuntu/+mirror/mirror.mawson.antarctica.org-releases')
+    >>> browser.getLink("Change details").click()
+    >>> print browser.title
+    Edit mirror Mawson Station...
+    >>> browser.getControl(name='field.content').value = ["ARCHIVE"]
+    >>> browser.getControl('Save').click()
+    >>> print find_tags_by_class(browser.contents, 'message')
+    [<p class="error message">There is 1 error.</p>,
+     <div class="message">Changing the mirror content type of country DNS
+     mirrors is forbidden.</div>]
+    >>> browser.open(
+    ...     'https://launchpad.dev/ubuntu/+mirror/mirror.mawson.antarctica.org-releases')
+    >>> browser.getLink("Change details").click()
+    >>> print browser.title
+    Edit mirror Mawson Station...
+    >>> browser.getControl(name='field.country').value = ['82'] # Germany.
+    >>> browser.getControl('Save').click()
+    >>> print find_tags_by_class(browser.contents, 'message')
+    [<p class="error message">There is 1 error.</p>,
+     <div class="message">Changing the country of country DNS mirrors is
+     forbidden.</div>]
+
 
 === Disabled mirrors ===
 
@@ -105,7 +256,7 @@
     'http://launchpad.dev/ubuntu/+unofficialmirrors'
 
     >>> print_mirrors_by_countries(browser.contents)
-    France: [(u'Invalid-mirror', u'http', u'2 Mbps', u'Last update unknown')]
+    France: [(u'Invalid-mirror', u'', u'http', u'2 Mbps', u'Last update unknown')]
 
 == Pending-review mirrors ==
 
@@ -133,8 +284,8 @@
     >>> browser.open('http://launchpad.dev/ubuntu/+pendingreviewmirrors')
     >>> print_mirrors_by_countries(browser.contents)
     Afghanistan:
-     [(u'Kabul LUG mirror', u'ftp', u'10 Gbps',
+     [(u'Kabul LUG mirror', u'', u'ftp', u'10 Gbps',
        u'Archive')]
     United Kingdom:
-      [(u'Random-releases-mirror', u'http', u'100 Mbps',
+      [(u'Random-releases-mirror', u'', u'http', u'100 Mbps',
         u'CD Image')]
 
=== modified file 'lib/lp/registry/templates/distributionmirror-index.pt'
--- lib/lp/registry/templates/distributionmirror-index.pt	2009-12-08 15:51:34 +0000
+++ lib/lp/registry/templates/distributionmirror-index.pt	2010-01-05 15:14:20 +0000
@@ -76,6 +76,15 @@
     </div>
     </div>
     </div>
+    <div tal:condition="context/country_dns_mirror">
+      <div class="portlet">
+        <h2>Country DNS mirror</h2>
+        <p>
+          This mirror is the country DNS mirror for <span
+          tal:content="context/country/name" />.
+        </p>
+      </div>
+    </div>
 
     <div class="portlet"
          id="last-probe"
 
=== modified file 'lib/lp/registry/templates/distributionmirror-macros.pt'
--- lib/lp/registry/templates/distributionmirror-macros.pt	2009-12-09 19:41:23 +0000
+++ lib/lp/registry/templates/distributionmirror-macros.pt	2010-01-05 15:14:20 +0000
@@ -19,6 +19,7 @@
       <tr class="highlighted">
         <th colspan="2" style="text-align: left"
             tal:content="country_and_mirrors/country" />
+        <th align="right" />
         <th style="text-align: left"
             tal:content="country_and_mirrors/throughput"/>
         <th style="text-align: left" tal:condition="show_mirror_type">
@@ -36,6 +37,10 @@
           <a tal:attributes="href mirror/fmt:url"
              tal:content="mirror/title">Mirror Name</a>
         </td>
+        <td align="right">
+          <img tal:condition="mirror/country_dns_mirror" src="/@@/favourite-yes"
+            title="Country DNS mirror" />
+        </td>
         <td>
           <a tal:condition="mirror/http_base_url"
              tal:attributes="href mirror/http_base_url">http</a>
@@ -65,10 +70,11 @@
 
     </tal:country_and_mirrors>
     <tr class="highlighted">
-      <th colspan="5" style="text-align: left; font-weight: bold;">Total</th>
+      <th colspan="6" style="text-align: left; font-weight: bold;">Total</th>
     </tr>
     <tr>
       <td colspan="2" />
+      <td />
       <td style="text-align: left; font-weight: bold;"
           tal:content="total_throughput" />
       <td tal:condition="show_mirror_type"></td>