Modified custom_arg message_id for compatibility with SendGrid webhook #109
Conversation
medmunds
left a comment
medmunds
left a comment
There was a problem hiding this comment.
Thanks for this -- it looks great. A few minor comments inline.
We should also update test_sendgrid_webhooks to switch to the new anymail_id field, and to cover the original problem case, where SendGrid gives the webhook a different smtp-id.
anymail/backends/sendgrid.py
Outdated
| # record, we can supply the 'smtp-id' field for any events missing it. | ||
| self.data.setdefault("custom_args", {})["smtp-id"] = self.message_id | ||
| # Set anymail-id to track our custom message ID | ||
| self.data.setdefault("custom_args", {})["anymail-id"] = self.message_id |
There was a problem hiding this comment.
I'd maybe go with "anymail_id" (underscore), to parallel all other webhook data fields. ("smtp-id" is the outlier.)
anymail/webhooks/sendgrid.py
Outdated
| metadata = {} | ||
|
|
||
| anymail_id = esp_event.get('anymail-id', None) | ||
| smtp_id = esp_event.get('smtp-id', None) |
There was a problem hiding this comment.
This could be simplified to message_id = esp_event.get('anymail_id', esp_event.get('smtp-id')).
(Use 'anymail_id' if present, else use 'smtp-id' for backwards compatibility.)
|
Updated based on further discussion in #108. Removed a concern for |
2 participants
Based our discussion in #108.
According to SendGrid support, the
Message-IDandsmtp-idare both used by SendGrid to set their unique message ID. Based on the conversation with SendGrid support we should use a custom arg with a key named something other thanMessage-IDorsmtp-id.Tried to set this up to ensure backwards compatibility for users who upgrade so that the
smtp-idstill gets referenced if theanymail-idis not present in the webhook data.