commit 94a2e9d7abbf96e7d5fb55375aa7db36aa12a2e5 Author: Damian Johnson atagar@torproject.org Date: Wed Nov 19 08:56:48 2014 -0800
Don't shell out to send emails
I'm glad our recent DocTor change forced me to revert my earlier hack to get around our OOM errors. On reflection it was stupid.
We shelled out to 'mail' because that's what the java DocTor implementation did, and I wasn't really sure how to interact with the local mail daemon. But that's dump - it's simply a local SMTP daemon so we can simply use the builtin module.
This avoids shelling out, and by extension both gets rid of an ugly hack and avoids forking our process (which triggered the OOM errors). --- consensus_health_checker.py | 45 +++---------------- descriptor_checker.py | 4 +- sybil_checker.py | 4 +- util.py | 100 ++++++++----------------------------------- 4 files changed, 27 insertions(+), 126 deletions(-)
diff --git a/consensus_health_checker.py b/consensus_health_checker.py index 26b4fc7..b4be10b 100755 --- a/consensus_health_checker.py +++ b/consensus_health_checker.py @@ -9,7 +9,6 @@ Performs a variety of checks against the present votes and consensus. import collections import datetime import os -import subprocess import time import traceback
@@ -214,30 +213,6 @@ def rate_limit_notice(issue): def main(): start_time = time.time()
- if not TEST_RUN: - # Spawning a shell to run mail. We're doing this early because - # subprocess.Popen() calls fork which doubles the memory usage of our - # process. Hence we risk an OOM if this is done after loading gobs of - # descriptor data into memory. - - # TODO: The following doesn't work now that we're dynamically picking - # destionations based on the issues. We'll need to come up with another - # idea if this continues to bite us... - - #mail_process = subprocess.Popen( - # ['mail', '-E', '-s', EMAIL_SUBJECT, util.TO_ADDRESS], - # stdin = subprocess.PIPE, - # stdout = subprocess.PIPE, - # stderr = subprocess.PIPE, - #) - - bot_notice_process = subprocess.Popen( - ['mail', '-E', '-s', 'Announce or', 'tor-misc@commit.noreply.org'], - stdin = subprocess.PIPE, - stdout = subprocess.PIPE, - stderr = subprocess.PIPE, - ) - # loads configuration data
config = stem.util.conf.get_config("consensus_health") @@ -294,24 +269,16 @@ def main(): if TEST_RUN: print '\n'.join(map(str, issues)) else: - #stdout, stderr = mail_process.communicate('\n'.join(map(str, issues))) - #exit_code = mail_process.poll() - - #if exit_code != 0: - # raise ValueError("Unable to send email: %s" % stderr.strip()) + body = '\n'.join(map(str, issues)) + cc = [d.address for d in destinations.values() if d and not d.bcc] + bcc = [d.address for d in destinations.values() if d and d.bcc]
- cc_addresses = [d.address for d in destinations.values() if d and not d.bcc] - bcc_addresses = [d.address for d in destinations.values() if d and d.bcc] - - util.send(EMAIL_SUBJECT, body_text = '\n'.join(map(str, issues)), cc_destinations = cc_addresses, bcc_destinations = bcc_addresses) + util.send(EMAIL_SUBJECT, body = body, cc = cc, bcc = bcc)
# notification for #tor-bots
- stdout, stderr = bot_notice_process.communicate('\n'.join(['[consensus-health] %s' % issue for issue in issues])) - exit_code = bot_notice_process.poll() - - if exit_code != 0: - raise ValueError("Unable to send notice to #tor-bots: %s" % stderr.strip()) + body = '\n'.join(['[consensus-health] %s' % issue for issue in issues]) + util.send('Announce or', body = body, to = ['tor-misc@commit.noreply.org']) else: if issues: log.info("All %i issues were suppressed. Not sending a notification." % len(issues)) diff --git a/descriptor_checker.py b/descriptor_checker.py index 6cd5328..6b2afca 100755 --- a/descriptor_checker.py +++ b/descriptor_checker.py @@ -92,7 +92,7 @@ def main(): def send_email(subject, descriptor_type, query): try: timestamp = datetime.datetime.now().strftime("%m/%d/%Y %H:%M") - util.send(subject, body_text = EMAIL_BODY % (descriptor_type, query.download_url, timestamp, query.error), destination = util.ERROR_ADDRESS) + util.send(subject, body = EMAIL_BODY % (descriptor_type, query.download_url, timestamp, query.error), to = [util.ERROR_ADDRESS]) except Exception, exc: log.warn("Unable to send email: %s" % exc)
@@ -103,4 +103,4 @@ if __name__ == '__main__': except: msg = "descriptor_checker.py failed with:\n\n%s" % traceback.format_exc() log.error(msg) - util.send("Script Error", body_text = msg, destination = util.ERROR_ADDRESS) + util.send("Script Error", body = msg, to = [util.ERROR_ADDRESS]) diff --git a/sybil_checker.py b/sybil_checker.py index 3dada88..5a88e8b 100755 --- a/sybil_checker.py +++ b/sybil_checker.py @@ -93,7 +93,7 @@ def send_email(new_relays): body = EMAIL_BODY % len(new_relays) body += "\n".join(relay_entries)
- util.send(EMAIL_SUBJECT, body_text = body) + util.send(EMAIL_SUBJECT, body = body) except Exception, exc: log.warn("Unable to send email: %s" % exc)
@@ -141,4 +141,4 @@ if __name__ == '__main__': except: msg = "sybil_checker.py failed with:\n\n%s" % traceback.format_exc() log.error(msg) - util.send("Script Error", body_text = msg, destination = util.ERROR_ADDRESS) + util.send("Script Error", body = msg, to = [util.ERROR_ADDRESS]) diff --git a/util.py b/util.py index 10f10b7..9440d4c 100644 --- a/util.py +++ b/util.py @@ -5,19 +5,15 @@ Module for issuing email notifications to me via gmail. import logging import os import smtplib -import subprocess
-from email import Encoders from email.mime.multipart import MIMEMultipart from email.mime.text import MIMEText -from email.MIMEBase import MIMEBase
import stem.util.log
-FROM_ADDRESS = 'verelsa@gmail.com' -TO_ADDRESS = 'tor-consensus-health@lists.torproject.org' +FROM_ADDRESS = 'atagar@torproject.org' +TO_ADDRESSES = ['tor-consensus-health@lists.torproject.org'] ERROR_ADDRESS = 'atagar@torproject.org' -PASSWORD = None
def get_path(*comp): @@ -79,46 +75,15 @@ def log_stem_debugging(name): log.addHandler(handler)
-def send(subject, body_text = None, destination = TO_ADDRESS, cc_destinations = None, bcc_destinations = None): +def send(subject, body, to = TO_ADDRESSES, cc = None, bcc = None): """ Sends an email notification via the local mail application.
:param str subject: subject of the email :param str body_text: plaintext body of the email - :param str destination: location to send the email to - :param list cc_destinations: addresses for the cc field - :param list bcc_destinations: addresses for the bcc field - - :raises: **Exception** if the email fails to be sent - """ - - args = ['mail', '-E', '-s', subject] - - if cc_destinations: - args += ['-c', ','.join(cc_destinations)] - - if bcc_destinations: - args += ['-b', ','.join(bcc_destinations)] - - args.append(destination) - - process = subprocess.Popen(args, stdin = subprocess.PIPE, stdout = subprocess.PIPE, stderr = subprocess.PIPE) - stdout, stderr = process.communicate(body_text) - exit_code = process.poll() - - if exit_code != 0: - raise ValueError("Unable to send email: %s" % stderr.strip()) - - -def send_via_gmail(subject, body_text = None, body_html = None, attachment = None, destination = TO_ADDRESS): - """ - Sends an email notification via gmail. - - :param str subject: subject of the email - :param str body_text: plaintext body of the email - :param str body_html: html body of the email - :param str attachment: path of a file to attach - :param str destination: location to send the email to + :param list to: destinations for the to field + :param list cc: destinations for the cc field + :param list bcc: destinations for the bcc field
:raises: **Exception** if the email fails to be sent """ @@ -126,50 +91,19 @@ def send_via_gmail(subject, body_text = None, body_html = None, attachment = Non msg = MIMEMultipart('alternative') msg['Subject'] = subject msg['From'] = FROM_ADDRESS - msg['To'] = destination - - if body_text: - msg.attach(MIMEText(body_text, 'plain')) - - if body_html: - msg.attach(MIMEText(body_html, 'html')) - - if attachment: - part = MIMEBase('application', "octet-stream") - part.set_payload(open(attachment, "rb").read()) - Encoders.encode_base64(part) - part.add_header('Content-Disposition', 'attachment; filename="%s"' % os.path.basename(attachment)) - msg.attach(part) - - # send the message via the gmail SMTP server - server = smtplib.SMTP('smtp.gmail.com:587') - server.starttls() - server.login(FROM_ADDRESS, _get_password()) - server.sendmail(FROM_ADDRESS, [destination], msg.as_string()) - server.quit() + msg['To'] = ','.join(to)
+ destinations = to
-def _get_password(): - """ - Provides the password for our gmail account. This is expected to be in a - local 'gmail_pw' file. + if cc: + msg['Cc'] = ','.join(cc) + destinations += cc
- :returns: **str** with our gmail password + if bcc: + destinations += bcc
- :raises: **ValueError** if our password file is unavalable or can't be read - """ + msg.attach(MIMEText(body, 'plain'))
- global PASSWORD - - if PASSWORD is None: - pw_path = get_path('gmail_pw') - - if not os.path.exists(pw_path): - raise ValueError("Unable to determine our gmail password, '%s' doesn't exist" % pw_path) - - try: - PASSWORD = open(pw_path).read().strip() - except Exception, exc: - raise ValueError('Unable to determine our gmail password: %s' % exc) - - return PASSWORD + server = smtplib.SMTP('localhost') + server.sendmail(FROM_ADDRESS, destinations, msg.as_string()) + server.quit()