Simple programme - Just want to know whether this is correct way of coding

[Dave Angel]

guptha wrote:
> Hi group,
> This is my first programme in python ,I need to know whether my code
> is in the right path of performance
>
> I wrote a code using multithreading to send mails
>
> FROM = '....com'
>
> SUBJECT = 'This is the subject'
> MSGBODY = 'This the body of the message '
> MAILSERVER = 'mail....com'
> port = 25
> username = 'username'
> password = 'pass'
>
> # trim the strings of any leading or trailing spaces
> FROM = FROM.strip()
> SUBJECT = SUBJECT.strip()
> MSGBODY = MSGBODY.strip()
> MAILSERVER = MAILSERVER.strip()
> username = username.strip()
> password = password.strip()
>
>
> # set up email parameters
> msg = MIMEMultipart()
> msg['From'] = FROM
> msg['Subject'] = SUBJECT
>
> #--------------------------------------------------
> print "Starting Multi Thread Method"
>
> class MyThread(Thread):
>
>     def __init__(self, site,  FROM, MSGBODY):
>         Thread.__init__(self)
>         self.site = site
> 	self.FROM=FROM
> 	self.MSGBODY=MSGBODY
>
>     def run(self):
> 	#Connect to server
> 	print 'Connecting to mail server ', MAILSERVER
> 	try:
> 		s = smtplib.SMTP(MAILSERVER,port)
> 		print 'connected'
> 	#s.set_debuglevel(1)
> 	except:
> 		print 'ERROR: Unable to connect to mail server', sys.exc_info	()[0]
> 	 	sys.exit(1)
>
> 	#login to server
> 	if password <> '':
> 		print 'Logging into mail server'
> 		try:
> 			s.login(username,password)
> 		except:
> 			print 'ERROR: Unable to login to mail server', MAILSERVER
> 			print 'Please recheck your password'
> 			sys.exit(1)
> 	print "running for %s " %self.site
> 	print s
> 	s.sendmail(self.FROM, self.site, self.MSGBODY)
> 	print "from %s" %self.FROM
> 	print "msg %s" %self.MSGBODY
>  	print "Emailed for  site %s" %self.site
> 	s.quit()
> 	s.close()
>
>
> a= time.time()
> threads = []
>
> for site in listTo:
>     T = MyThread(site,FROM,MSGBODY)
>     threads.append(T)
>     T.start()
>
>
> for i in threads:
>     i.join()
>
> print "Took %s seconds" %str(time.time()-a)
>
> #-----------------------------------------------------
>
> The code Works fine ,but I doubt about the performance issue ,My
> intention is to send mails concurrently to large number of mail.
> 1.For every mail id i send It creates a new SMTP object,in case, if i
> send to 1000 or more ids
>       a) It obliviously creates that much SMPT connections ,is this a
> right approach .
> Thanks in Advance
>
>   
Any program that launches multiple threads is not a "simple program," 
especially for a first time user.

I don't know smtplib at all, so these questions may be off-base.  First, 
sendmail() takes as its second argument a list of to_addr, so why not 
send these all as a single operation?  If that would work, this would 
degenerate into a much simpler program.  More importantly, it should 
generate much less traffic between your machine and your smtp server, 
and probably less traffic between that server and the actual destination 
domains.  I have to assume that if it's a single sendmail request, the 
server would then batch-send them to each unique domain in the to_addrs 
list.    Anyway, what if the message body be a couple of megabytes, and 
you're sending it to 100 people?  Sending a single message with a list 
in to_list would save tons of time.

Second, if you do have to send them all separately (for example, if you 
had different mail_options, which you don't yet), then I question the 
need or workability of using a separate thread for all of them.  Trying 
to open  1000 threads is very resource hungry,  Trying to open that many 
smtp connections is even worse.  One of these is likely to fail.  And 
I'm guessing that beyond 10 or so, it'd run at least as fast with a 
different approach.


If I had to do something like this, I'd expect to wind up with some 
number (say 10) of threads, and a queue of things for them to do.  So if 
you had 1000 things, each thread would do approximately 100 of them.  
Now, sharing a queue like that is more work, with more things to mess 
up.  Currently your threads share only read-only data, which makes 
threads pretty straightforward.