search

brack3t / Djrill

[INACTIVE/UNMAINTAINED] Djrill is an email backend and new message class for Django users that want to take advantage of the Mandrill transactional email service from MailChimp.
BSD 3-Clause "New" or "Revised" License
319 stars 64 forks source link

Single connection v2.0 dev #96

Closed Akamad007 closed 9 years ago

Akamad007 commented 9 years ago

This is a fix for the following issue. I have tested it on my local machine using celery. It worked fine. http://stackoverflow.com/questions/30982717/in-django-send-mass-mail-creates-a-new-http-connection- everytime-it-sends-a-mail

medmunds commented 9 years ago

Thanks for contributing to Djrill, @Akamad007. This is a nice improvement for large sends.

Two things:

  1. Would you mind squashing your commits to get rid of "removed build files" and "removed eclipse hidden files"? (Longer explanation.)
  2. This change breaks the tests -- not because your code is wrong, but because the tests are wrong after your change. The issue is that DjrillBackendMockAPITestCase patches requests.post, but with your change we're now calling requests.Session.post.
Akamad007 commented 9 years ago

@medmunds Should I change the tests to accommodate "session" ? I have removed the comments.

medmunds commented 9 years ago

@Akamad007 it would be great if you could update the tests to use session. Here's how to run them locally (you may need to install mock and six). If you run into any problems with it or it starts to become too much work, just let me know and I'm happy to figure it out.

Also, it looks like your build files and eclipse hidden files have reappeared in this pull request (check the "files changed" tab). We actually don't want those -- we only want the changes you made to the Djrill backend. But we want it all to show up as a single commit (check the "commits" tab). You can fix that by "squashing" your three earlier commits into one with git-rebase. (The links my earlier comment show how. Sorry if I wasn't clear about that.)

Akamad007 commented 9 years ago

@medmunds I am currently working on the tests part. It is taking more time than expected as I am new to mock. I will keep you updated regarding this. If I am unable to do it, I will let you know. Thanks :)

medmunds commented 9 years ago

Covered by #98