Merge lp:~mabac/launchpad-work-items-tracker/better-error-contacts into lp:~linaro-automation/launchpad-work-items-tracker/linaro

[Mattias Backman]

Hi,

This branch changes the format of the config file so that we specify contacts for receiving error emails per project.

The current way is to match a blueprint name with a regexp from the config file to find out where to send those emails. This does miss quite a few errors which we instead see in the log files and email to the Infrastructure errors list.

There is a corresponding config file in lp:~mabac/launchpad-work-items-tracker/better-config

Thanks,

Mattias

[Guilherme Salgado (salgado)]

Yay! Thanks for working on this, Mattias.

On 03/02/12 08:05, Mattias Backman wrote:
>
> === modified file 'collect'
> --- collect 2011-12-15 18:37:53 +0000
> +++ collect 2012-02-03 11:04:28 +0000
> @@ -667,9 +667,10 @@
> def send_error_mails(cfg):
> '''Send data_errors to contacts.
>
> - Data error contacts are defined in the configuration in the "error_contact"
> - map (which assigns a regexp over spec names to a list of email addresses).
> - If no match is found, the error goes to stderr.
> + Data error contacts are defined in the configuration in the
> + "project_notification_addresses" map (which assigns project names to a list
> + of email addresses). If no address list for a project is found, the error
> + goes to stderr.

I wonder if we should make the contact address required...

The rest looks pretty good; can't wait to stop receiving all those
warnings on infra-errors!

[Mattias Backman (mabac)]

On Fri, Feb 3, 2012 at 1:52 PM, Guilherme Salgado
<email address hidden> wrote:
> Yay!  Thanks for working on this, Mattias.

No problem. I'm also tired of getting spam about typos. ;)

>
> On 03/02/12 08:05, Mattias Backman wrote:
>>
>> === modified file 'collect'
>> --- collect   2011-12-15 18:37:53 +0000
>> +++ collect   2012-02-03 11:04:28 +0000
>> @@ -667,9 +667,10 @@
>>  def send_error_mails(cfg):
>>      '''Send data_errors to contacts.
>>
>> -    Data error contacts are defined in the configuration in the "error_contact"
>> -    map (which assigns a regexp over spec names to a list of email addresses).
>> -    If no match is found, the error goes to stderr.
>> +    Data error contacts are defined in the configuration in the
>> +    "project_notification_addresses" map (which assigns project names to a list
>> +    of email addresses). If no address list for a project is found, the error
>> +    goes to stderr.
>
> I wonder if we should make the contact address required...

Umm, yes we shouldn't have project groups without contact addresses.
But I'd hate to break the update because of a bad config, so I'd
prefer not to bail on it. Do you think we can handle this when
reviewing config changes?

>
> The rest looks pretty good; can't wait to stop receiving all those
> warnings on infra-errors!
>
>
> --
> https://code.launchpad.net/~mabac/launchpad-work-items-tracker/better-error-contacts/+merge/91410
> You are the owner of lp:~mabac/launchpad-work-items-tracker/better-error-contacts.

[Guilherme Salgado (salgado)]

On 03/02/12 11:28, Mattias Backman wrote:
> On Fri, Feb 3, 2012 at 1:52 PM, Guilherme Salgado
> <email address hidden> wrote:
>> Yay! Thanks for working on this, Mattias.
>
> No problem. I'm also tired of getting spam about typos. ;)
>
>>
>> On 03/02/12 08:05, Mattias Backman wrote:
>>>
>>> === modified file 'collect'
>>> --- collect 2011-12-15 18:37:53 +0000
>>> +++ collect 2012-02-03 11:04:28 +0000
>>> @@ -667,9 +667,10 @@
>>> def send_error_mails(cfg):
>>> '''Send data_errors to contacts.
>>>
>>> - Data error contacts are defined in the configuration in the "error_contact"
>>> - map (which assigns a regexp over spec names to a list of email addresses).
>>> - If no match is found, the error goes to stderr.
>>> + Data error contacts are defined in the configuration in the
>>> + "project_notification_addresses" map (which assigns project names to a list
>>> + of email addresses). If no address list for a project is found, the error
>>> + goes to stderr.
>>
>> I wonder if we should make the contact address required...
>
> Umm, yes we shouldn't have project groups without contact addresses.
> But I'd hate to break the update because of a bad config, so I'd
> prefer not to bail on it. Do you think we can handle this when
> reviewing config changes?

We don't have config changes too often, do we? And in most cases I've
seen they'd be just a matter of extending the list of projects of an
existing class, so wouldn't break anything. We could also add a small
test script to the config branch, just to do some sanity checking, but
that we can probably save for later.

[Mattias Backman (mabac)]

On Fri, Feb 3, 2012 at 3:52 PM, Guilherme Salgado
<email address hidden> wrote:
> On 03/02/12 11:28, Mattias Backman wrote:
>> On Fri, Feb 3, 2012 at 1:52 PM, Guilherme Salgado
>> <email address hidden> wrote:
>>> Yay!  Thanks for working on this, Mattias.
>>
>> No problem. I'm also tired of getting spam about typos. ;)
>>
>>>
>>> On 03/02/12 08:05, Mattias Backman wrote:
>>>>
>>>> === modified file 'collect'
>>>> --- collect   2011-12-15 18:37:53 +0000
>>>> +++ collect   2012-02-03 11:04:28 +0000
>>>> @@ -667,9 +667,10 @@
>>>>  def send_error_mails(cfg):
>>>>      '''Send data_errors to contacts.
>>>>
>>>> -    Data error contacts are defined in the configuration in the "error_contact"
>>>> -    map (which assigns a regexp over spec names to a list of email addresses).
>>>> -    If no match is found, the error goes to stderr.
>>>> +    Data error contacts are defined in the configuration in the
>>>> +    "project_notification_addresses" map (which assigns project names to a list
>>>> +    of email addresses). If no address list for a project is found, the error
>>>> +    goes to stderr.
>>>
>>> I wonder if we should make the contact address required...
>>
>> Umm, yes we shouldn't have project groups without contact addresses.
>> But I'd hate to break the update because of a bad config, so I'd
>> prefer not to bail on it. Do you think we can handle this when
>> reviewing config changes?
>
> We don't have config changes too often, do we?  And in most cases I've
> seen they'd be just a matter of extending the list of projects of an
> existing class, so wouldn't break anything. We could also add a small
> test script to the config branch, just to do some sanity checking, but
> that we can probably save for later.

I was actually thinking that we should have some validator script.
Didn't have time to add that now though.

We're testing the changes on staging atm:
http://status.linaro.org/staging/update.log.txt

>
>
> --
> https://code.launchpad.net/~mabac/launchpad-work-items-tracker/better-error-contacts/+merge/91410
> You are the owner of lp:~mabac/launchpad-work-items-tracker/better-error-contacts.

=== modified file 'collect'
--- collect	2011-12-15 18:37:53 +0000
+++ collect	2012-02-03 11:04:28 +0000
@@ -667,9 +667,10 @@
 def send_error_mails(cfg):
     '''Send data_errors to contacts.
 
-    Data error contacts are defined in the configuration in the "error_contact"
-    map (which assigns a regexp over spec names to a list of email addresses).
-    If no match is found, the error goes to stderr.
+    Data error contacts are defined in the configuration in the 
+    "project_notification_addresses" map (which assigns project names to a list
+    of email addresses). If no address list for a project is found, the error
+    goes to stderr.
     '''
     global error_collector
 
@@ -678,15 +679,14 @@
 
     dbg('mailing %i data errors' % len(error_collector.errors))
     for error in error_collector.errors:
-        for pattern, addresses in cfg['error_contact'].iteritems():
-            if (error.get_blueprint_name() is not None
-                and re.search(pattern, error.get_blueprint_name())):
-                dbg('spec %s matches error_contact pattern "%s", mailing to %s' % (error.get_blueprint_name(),
-                    pattern, ', '.join(addresses)))
-                for a in addresses:
-                    emails.setdefault(a, '')
-                    emails[a] += error.format_for_display() + '\n'
-                break
+        project_name = error.get_project_name()
+        if project_name is not None:
+            addresses = cfg['project_notification_addresses'][project_name]
+            dbg('spec %s is targetted to "%s", mailing to %s' % (error.get_blueprint_name(),
+                                                                 project_name, ', '.join(addresses)))
+            for a in addresses:
+                emails.setdefault(a, '')
+                emails[a] += error.format_for_display() + '\n'
         else:
             print >> sys.stderr, error.format_for_display(), '(no error_contact pattern)'
 
 
=== modified file 'lpworkitems/error_collector.py'
--- lpworkitems/error_collector.py	2011-06-08 19:30:24 +0000
+++ lpworkitems/error_collector.py	2012-02-03 11:04:28 +0000
@@ -48,6 +48,10 @@
         """Get the name of the blueprint, or None if not a blueprint."""
         return None
 
+    def get_project_name(self):
+        """Get the name of the project, or None if not a project."""
+        return None
+
     def format_for_display(self):
         """Produce a string representation of the Error.
 
@@ -84,6 +88,9 @@
     def get_blueprint_name(self):
         return self.blueprint.name
 
+    def get_project_name(self):
+        return self.blueprint.url.split('/')[-3]
+
 
 class BlueprintURLError(Error):
     """A deprecated class for backwards-compatibility.
@@ -101,6 +108,9 @@
     def get_blueprint_name(self):
         return self.blueprint_url.split('/')[-1]
 
+    def get_project_name(self):
+        return self.blueprint_url.split('/')[-3]
+
 
 class MilestoneError(Error):