Merge lp:~wgrant/launchpad/silence-of-the-prober-ii into lp:launchpad

[William Grant]

[r=stevenk,thumper][bug=370329,502842,721028] Fix a few errors in the distribution mirror prober.

This branch fixes three issues in the mirror prober that make my inbox sad:

 - Sending a failure email for a mirror with a deactivated owner causes an exception due to a lack of To addresses. I've fixed this by first checking if the set of destination addresses is non-empty.
 - Some mirrors are too cool for 404s, instead redirecting to a special Not Found document. The prober logged these incidents as errors that required investigation. I've added them as an expected error alongside other invalid redirects and response codes, so they behave correctly like 404s.
 - Some non-Ubuntu mirrors exist, but we do not support probing them. This is a routine situation, not requiring any special attention. I have reduced its warning from INFO to DEBUG due to its unimportance.

[Steve Kowalik (stevenk)]

This change looks great to me, and awesome work fixing 3 critical bugs.

review: Approve (code*)

[Tim Penhey (thumper)]

review: Approve (code)

[Curtis Hovey (sinzui)]

I am very excited to see these issues fixed. Thanks William.

=== modified file 'lib/lp/registry/model/distributionmirror.py'
--- lib/lp/registry/model/distributionmirror.py	2011-01-20 20:27:43 +0000
+++ lib/lp/registry/model/distributionmirror.py	2011-02-18 05:04:57 +0000
@@ -387,7 +387,8 @@
 
         if notify_owner:
             owner_address = get_contact_email_addresses(self.owner)
-            simple_sendmail(fromaddress, owner_address, subject, message)
+            if len(owner_address) > 0:
+                simple_sendmail(fromaddress, owner_address, subject, message)
 
     def newProbeRecord(self, log_file):
         """See IDistributionMirror"""

=== modified file 'lib/lp/registry/scripts/distributionmirror_prober.py'
--- lib/lp/registry/scripts/distributionmirror_prober.py	2010-12-24 17:08:10 +0000
+++ lib/lp/registry/scripts/distributionmirror_prober.py	2011-02-18 05:04:57 +0000
@@ -565,6 +565,7 @@
         BadResponseCode,
         ConnectionSkipped,
         ProberTimeout,
+        RedirectToDifferentFile,
         UnknownURLSchemeAfterRedirect,
         )
 
@@ -848,7 +849,7 @@
             # Some people registered mirrors on distros other than Ubuntu back
             # in the old times, so now we need to do this small hack here.
             if not mirror.distribution.full_functionality:
-                self.logger.info(
+                self.logger.debug(
                     "Mirror '%s' of distribution '%s' can't be probed --we "
                     "only probe Ubuntu mirrors."
                     % (mirror.name, mirror.distribution.name))

=== modified file 'lib/lp/registry/tests/test_distributionmirror.py'
--- lib/lp/registry/tests/test_distributionmirror.py	2011-01-20 17:54:24 +0000
+++ lib/lp/registry/tests/test_distributionmirror.py	2011-02-18 05:04:57 +0000
@@ -23,6 +23,7 @@
 from lp.registry.model.distributionmirror import DistributionMirror
 from lp.services.mail import stub
 from lp.services.worlddata.interfaces.country import ICountrySet
+from lp.testing import login_as
 from lp.testing.factory import LaunchpadObjectFactory
 
 
@@ -208,6 +209,25 @@
         self.failUnlessEqual(len(stub.test_emails), 0)
         stub.test_emails = []
 
+    def test_no_email_sent_to_uncontactable_owner(self):
+        # If the owner has no contact address, only the mirror admins are
+        # notified.
+        mirror = self.cdimage_mirror
+        login_as(mirror.owner)
+        # Deactivate the mirror owner to remove the contact address.
+        mirror.owner.deactivateAccount("I hate mirror spam.")
+        login_as(mirror.distribution.mirror_admin)
+        # Clear out notifications about the new team member.
+        transaction.commit()
+        stub.test_emails = []
+
+        # Disabling the mirror results in a single notification to the
+        # mirror admins.
+        self.factory.makeMirrorProbeRecord(mirror)
+        mirror.disable(notify_owner=True, log="It broke.")
+        transaction.commit()
+        self.failUnlessEqual(len(stub.test_emails), 1)
+
 
 class TestDistributionMirrorSet(unittest.TestCase):
     layer = LaunchpadFunctionalLayer

=== modified file 'lib/lp/registry/tests/test_distributionmirror_prober.py'
--- lib/lp/registry/tests/test_distributionmirror_prober.py	2010-12-24 10:03:15 +0000
+++ lib/lp/registry/tests/test_distributionmirror_prober.py	2011-02-18 05:04:57 +0000
@@ -57,6 +57,7 @@
     RedirectAwareProberFactory,
     RedirectAwareProberProtocol,
     RequestManager,
+    RedirectToDifferentFile,
     restore_http_proxy,
     should_skip_host,
     UnknownURLSchemeAfterRedirect,
@@ -666,11 +667,13 @@
                 BadResponseCode,
                 ProberTimeout,
                 ConnectionSkipped,
+                RedirectToDifferentFile,
                 UnknownURLSchemeAfterRedirect,
                 ]))
         exceptions = [BadResponseCode(str(httplib.NOT_FOUND)),
                       ProberTimeout('http://localhost/', 5),
                       ConnectionSkipped(),
+                      RedirectToDifferentFile('/foo', '/bar'),
                       UnknownURLSchemeAfterRedirect('https://localhost')]
         for exception in exceptions:
             failure = callbacks.ensureOrDeleteMirrorCDImageSeries(