827856 - Suppress the weekly War on Orange summary email to dev.tree-management if the OF is 0.0

Status: RESOLVED FIXED

(Tree Management Graveyard :: OrangeFactor, defect)

Description

[Ed Morley [:emorley]]

...since something must have broken.

Bonus points if we email someone about the failure too :-)

Comment 1

[Ed Morley [:emorley]]

(It happened again this week)

Comment 2

[Ed Morley [:emorley]]

Attachment: Patch v1

Comment 3

[Mark Côté [:mcote]]

Comment on attachment 699258 [details] [diff] [review]
Patch v1

This looks good, but can you move the check above the previous_stats assignment?  No point in getting last week's stats if this week's don't make any sense.

Didn't feel like adding in an email alert? :) Should be simple; I would just add a new (optional) entry to the conf file, maybe [email] admin, which contains a list of emails to be alerted when this problem occurs.

Comment 4

[Ed Morley [:emorley]]

Attachment: Patch v2

How about this?

Also makes test mode return early, since we previously would have still sent the newsgroup post etc.

Comment 5

[Mark Côté [:mcote]]

Comment on attachment 702350 [details] [diff] [review]
Patch v2

Review of attachment 702350 [details] [diff] [review]:
-----------------------------------------------------------------

::: woo_mailer.py
@@ +232,5 @@
> +        print 'To: %s' % ', '.join(mail_dest)
> +        print 'Subject: %s' % subject
> +        print
> +        print text
> +        return

The newsgroup message was *not* being sent in test mode; it was just being displayed so we could see headers and such and verify that it would have been sent.  I think I'd like to keep that behaviour.

@@ +239,5 @@
> +        sys.stderr.write('This week\'s orange count is zero! Something must have broken :-( Sending email to admin only...\n')
> +        sendemail(from_addr=from_address, to_addrs=admin_email,
> +                  subject=subject, username=mail_username,
> +                  password=mail_password, text_data=text,
> +                  server=mail_server, port=mail_port, use_ssl=mail_ssl)

I think we need an additional message in here, preferably in the subject line, alerting us that something bad happened.  Since this just looks like the regular email report, it would be too easy to ignore or miss.

Comment 6

[Ed Morley [:emorley]]

(In reply to Mark Côté ( :mcote ) from comment #5)
> The newsgroup message was *not* being sent in test mode; it was just being
> displayed so we could see headers and such and verify that it would have
> been sent.  I think I'd like to keep that behaviour.

Oh I totally missed the second options.test_mode conditional inside the newsgroup block. I thought it had been omitted.

Yeah in that case, patch v2 doesn't make sense.

Comment 7

[Ed Morley [:emorley]]

Attachment: Patch v3

* If we're using test_mode continues as normal, since we'll want to have the results printed as is
* Otherwise if not, and OF <= 0 we email the admin(s) with a more noticeable subject & we exit(1) + print the error message (thanks to the trick at http://docs.python.org/2/library/sys.html#sys.exit)
* Also removes unused import 'os' (now zero pyflakes warnings)

Comment 8

[Mark Côté [:mcote]]

Comment on attachment 702350 [details] [diff] [review]
Patch v2

Perfect, thanks. :) I'll deploy it as soon as you check it in.

Comment 9

[Ed Morley [:emorley]]

Thank you :-)

https://hg.mozilla.org/automation/orangefactor/rev/8f16c34e7d17

Comment 10

[Mark Côté [:mcote]]

Comment on attachment 702350 [details] [diff] [review]
Patch v2

r+ed the wrong patch here....

Comment 11

[Mark Côté [:mcote]]

Comment on attachment 702553 [details] [diff] [review]
Patch v3

Carry over this r+ with the removed unused import.

Comment 12

[Ed Morley [:emorley]]

In production.