Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix TypeError that occures during messages sending #69

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nautics889
Copy link

@nautics889 nautics889 commented Feb 8, 2020

I encountered a TypeError error during sending messages, because it tried to add an integer and a list in tasks.py. I have been searching for the reason and found a little inconsistency with what Django's documentation suggests:

This method (send_messages(email_messages)) receives a list of EmailMessage instances and returns the number of successfully delivered messages.

So it looked like there was a problem in CeleryEmailBackend.send_messages in backends.py, but when i tried to fix that method and implement returning number of sent messages instead of a list with async results, several tests were failed as the result. Probably, it had had a side-effect for some other cases. So i decided to add a type checking in the tasks.py.

Note: probably you may also want to look at CeleryEmailBackend.send_messages

@nautics889 nautics889 requested review from pmac and pmclanahan and removed request for pmclanahan February 8, 2020 18:20
@pmac
Copy link
Collaborator

pmac commented Sep 23, 2020

The send_messages method used in tasks.py is one of Django's built in one or a custom one someone else writes. It should not be CeleryEmailBackend.send_messages because that's the one that has caused the task to be run. You make a good point that the send_messages in the celery backend should probably return an integer like the Django docs suggest, but for now it doesn't so that users' code can check the results of the tasks if they need to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants