RyanBalfanz / django-sendgrid

SendGrid for Django
http://pypi.python.org/pypi/django-sendgrid
97 stars 26 forks source link

Multiple objects return from get() query #48

Open slezica opened 11 years ago

slezica commented 11 years ago

NOTE: the following error arises in a slightly modified implementation, so I can't guarantee it exists in this codebase, and does not stop the application from sending the e-mails.

Stack trace:

File "/app/.heroku/venv/lib/python2.7/site-packages/sendgrid/message.py", line 121, in send
    sendgrid_email_sent.send(sender=self, message=self, response=response)
  File "/app/.heroku/venv/lib/python2.7/site-packages/django/dispatch/dispatcher.py", line 172, in send
    response = receiver(signal=self, sender=sender, **named)
  File "/app/.heroku/venv/lib/python2.7/site-packages/sendgrid/models.py", line 109, in save_email_message
    argument, argumentCreated = Argument.objects.get_or_create(key=k)
  File "/app/.heroku/venv/lib/python2.7/site-packages/django/db/models/manager.py", line 134, in get_or_create
    return self.get_query_set().get_or_create(**kwargs)
  File "/app/.heroku/venv/lib/python2.7/site-packages/django/db/models/query.py", line 442, in get_or_create
    return self.get(**lookup), False
  File "/app/.heroku/venv/lib/python2.7/site-packages/django/db/models/query.py", line 368, in get
    % (self.model._meta.object_name, num, kwargs))
MultipleObjectsReturned: get() returned more than one Argument -- it returned 2! Lookup parameters were {'key': 'message_id'}

I'll make time to dig up the relevant database rows and, if I find it's not my fault, post minimal code to reproduce the issue.

RyanBalfanz commented 11 years ago

Looks like you have multiple sendgrid.models.Arguments (https://github.com/RyanBalfanz/django-sendgrid/blob/develop/sendgrid/models.py#L157) of the same key.

There should probably be a unique=True on Argument.key but there is not in the current release. I'd like to better understand how multiple Arguments of that key were created in the first place before adding that constraint in a later release.

FWIW, I've sent well over a million emails and haven't exhibited this behavior, which makes me think it's probably okay to add that constraint. However, in your case the migration should fail unless you manually migrate your data.

RyanBalfanz commented 11 years ago

@slezica I quickly threw together a tool to help with the data migration (see 904f21488e69aaf2f33b54c85f11ca1224256552)

slezica commented 11 years ago

Wow, swift action! Thanks a lot, Ryan!

RyanBalfanz commented 11 years ago

Thank @hoddez and myself once it's released ;) We're still doing some testing and such…