patternpythonMinor
Attach list of files and email using secure connection
Viewed 0 times
secureemailattachfilesusingandlistconnection
Problem
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?)
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()Solution
In general, the code looks good. I just have a few miscellaneous remarks.
Code organization
-
The parameters are hard-coded, so the function is of limited utility. The function could accept parameters with defaults:
Mail content
-
Concatenating file paths using
Code organization
- Function names should be named
lower_case_with_underscoresby 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
bodyas 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 theMIMETextusingEncoders.encode_quopri().
Code Snippets
def compose_email(from='sender@example.com',
to=['recipient_1@example.com','recipient_2@example.com'],
attachments=[])
…
def send_email(msg, username, password)
…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']
])Context
StackExchange Code Review Q#84218, answer score: 4
Revisions (0)
No revisions yet.