[tor-commits] [doctor/master] Don't shell out to send emails

atagar at torproject.org atagar at torproject.org
Wed Nov 19 16:59:59 UTC 2014


commit 94a2e9d7abbf96e7d5fb55375aa7db36aa12a2e5
Author: Damian Johnson <atagar at 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 at 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 at 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 at gmail.com'
-TO_ADDRESS = 'tor-consensus-health at lists.torproject.org'
+FROM_ADDRESS = 'atagar at torproject.org'
+TO_ADDRESSES = ['tor-consensus-health at lists.torproject.org']
 ERROR_ADDRESS = 'atagar at 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()



More information about the tor-commits mailing list