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

Add mandrill_response property to EmailMessage #55

Closed erichennings closed 10 years ago

erichennings commented 10 years ago

Add the Mandrill send API response to EmailMessage as a property, mandrill_response, when a message is sent. For errors, set mandrill_response to None. Add tests & docs.

medmunds commented 10 years ago

@erichennings thanks -- tests, docs, and everything!

One small issue: the tests are failing under Python 3 (no more u'strings'). If you have a chance to fix that, great; otherwise I can probably clean it up and merge in the next few days.

(Sorry for the delayed response -- just getting back from vacation.)

medmunds commented 10 years ago

Released in Djrill 0.8. Closes #54.

erichennings commented 10 years ago

Thanks! I appreciate the very quick release.

Another minor tweak I was considering submitting - on my setup with Apache / mod_wsgi I have a problem the webhook doesn't work because the HEAD request that Mandrill sends to verify the webhook gets translated by mod_wsgi to a GET, which fails with a method not allowed exception.

Here's a blog post referring to the HEAD to GET translation: ... Apache/mod_wsgi has recognised this problem for a while though and as a result when it detects that an output filter has been registered with Apache that processes the response content, it will forcibly change 'REQUEST_METHOD' from 'HEAD' to 'GET' even before it is passed into the WSGI stack. The WSGI application as far as it can tell got a GET request and is none the wiser. ... http://blog.dscpl.com.au/2009/10/wsgi-issues-with-http-head-requests.html

Renaming the head method to get in DjrillWebhookView fixes this (so that both get and head are supported).

Does that make sense to you?

On Sun, Jan 12, 2014 at 11:30 AM, Mike Edmunds notifications@github.comwrote:

Released in Djrill 0.8. Closes #54https://github.com/brack3t/Djrill/issues/54.

— Reply to this email directly or view it on GitHubhttps://github.com/brack3t/Djrill/pull/55#issuecomment-32131197 .

gabejackson commented 10 years ago

awesome contribution, thanks!

medmunds commented 10 years ago

Hey, @erichennings, just noticed your comments about Apache mod_wsgi above.

I'm not sure Djrill should be working around oddnesses in specific WSGI implementations. We risk breaking it for people using other WSGIs that don't have the behavior you cite. (Also, wouldn't you have to add a similar workaround to every other package you're using that expects HEAD requests?)

It might be reasonable to make the DjrillWebhookView handle a GET the same as a HEAD (while still leaving HEAD handling in place). Suggest opening this as a separate issue.