5
\$\begingroup\$

Can I use this function to reliably attach a list of excel files to an email and create a secure connection to transmit said email? My python programming is self taught and I put this function together with a lot of google searches. (This function works, I use it, but I would like to have someone with more experience look it through, if possible)

Where can I improve my code? (Am I using the correct modules?)

import datetime
import smtplib
from email.MIMEMultipart import MIMEMultipart
from email.MIMEText import MIMEText
from email.MIMEBase import MIMEBase
from email import Encoders

sMDY = datetime.date.today().strftime('%m/%d/%y')
lpath = 'C:/Path/to/my/excel/files/'
flist = ['excelfile_1.xlsx', 'excelfile_2.xlsx']

def EMAIL_FILES(flist):
    uname = 'username'
    pword = 'password'

    emailfrom = 'sender@domain.com'
    emailto = [
        'recipient_1@domain.com',
        'recipient_2@domain.com',
        ]

    body = ["The information transmitted is ..."]
    subject = ('Subject with date %s' % sMDY)

    msg = MIMEMultipart()
    msg['From'] = emailfrom
    msg['To'] = ', '.join(emailto)
    msg['Subject'] = subject

    msg.attach(MIMEText(''.join(body)))

    ### ATTACH FILES
    for item in flist:
        part = MIMEBase('application', "octet-stream")

        part.set_payload(open(lpath + item, "rb").read())

        Encoders.encode_base64(part)

        part.add_header('Content-Disposition',
                        'attachment; filename="%s"' % item)
        msg.attach(part)


    ### SECURE CONNECTION
    server = smtplib.SMTP('smtp.domain.com:25')
    server.ehlo()
    server.starttls()
    server.ehlo()
    server.login(uname, pword)

    server.sendmail(emailfrom, emailto, msg.as_string())
    server.quit()
\$\endgroup\$
3
\$\begingroup\$

In general, the code looks good. I just have a few miscellaneous remarks.

Code organization

  • Function names should be named lower_case_with_underscores by PEP 8.
  • I would split the message composition and sending into separate functions.
  • It is good practice to avoid embedding passwords into your source code. I suggest storing the account information in a YAML file, or at least as constants in a separate Python module.
  • The parameters are hard-coded, so the function is of limited utility. The function could accept parameters with defaults:

    def compose_email(from='sender@example.com',
                      to=['recipient_1@example.com','recipient_2@example.com'],
                      attachments=[])
        …
    
    def send_email(msg, username, password)
        …
    

Mail content

  • The MIME type for .xlsx files is application/vnd.openxmlformats-officedocument.spreadsheetml.sheet.
  • Concatenating file paths using os.path.join(lpath, item) would let you not have to worry about the path component separator. Personally, I would choose to pass full paths to the function instead, to avoid assuming that they are all in some hard-coded directory:

    def EMAIL_FILES(attachments):
        …
        for path in attachments:
            with open(path, 'rb') as f:
                part = MIMEBase('application', 'vnd.openxmlformats-officedocument.spreadsheetml.sheet')
                part.set_payload(f.read())
                part.add_header('Content-Disposition',
                    'attachment; filename="%s"' % os.path.basename(path))
                Encoders.encode_base64(part)
                msg.attach(part)
        …
    
    EMAIL_FILES([os.path.join('C:/Path/to/my/excel/files/', f) for f in
        ['excelfile_1.xlsx', 'excelfile_2.xlsx']
    ])
    
  • I'm puzzled by why you defined body as a list of strings, only to join them all into one long string. If the body contains non-trivial content, consider writing it as a """longstring""". Also, you may need to encode the MIMEText using Encoders.encode_quopri().
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.