0

I have a function that runs every hour with apscheduler, this verifies few conditions and according to this sends an email to an arrangement of emails

from mailsnake import MailSnake
mapi = MailSnake('XXXXXXXX', api='mandrill')
def functionEverHour():
     users=Users.objects.all()
     for users in users:
         if users.notificated==False:
             mapi.messages.send(message={'html':'sending emails', 'subject':'test', 'from_email':'tes@test.com', 'from_name':'example', 'to':[{'email':str(users.email)}]})

maybe putting this line (mapi = MailSnake('Xajnas12sjnjas', api='mandrill')) in the cycle avoid that kind of mistake?

what does exactly mapi = MailSnake ('Xajnas12sjnjas' api = 'mandrill') that opens a connection to mandrill and never closes and that's the reason I sent emails repetitive (is accumulative?)?

Thanks

LaBE
  • 117
  • 1
  • 9

1 Answers1

0

This may or not be an error, but why are you doing users in users? It's probably not going to cause an error, but the naming isn't the best in my opinion. My suggestion would be to use naming: all_users = User.objects.all() and users_to_notify in all_users:

Not sure why you're using MailSnake, but this is something mandrill's API handles pretty well.

this is my suggestion about how to change your code. One thing is your method is doing too much. It's handling both the logic of finding the users to notify as well as send the emails. It's best practice to have a function and objects, etc handling only one responsibility.

def get_users_to_notify():
    all_users=Users.objects.all() 
    users_to_notify = []
    for user in all_users:
        if user.notified==False:
            users_to_notify.append(user)
    return users_to_notify

def functionEverHour():
    users = get_users_to_notify()
    for user in user:
        mapi.messages.send(message={'html':'sending emails', 'subject':'test', 'from_email':'tes@test.com', 'from_name':'example', 'to':[{'email':str(users.email)}]})
TalkativeTree
  • 609
  • 5
  • 10