Django

Code

Ticket #6970 (closed: fixed)

Opened 8 months ago

Last modified 4 months ago

misleading error message if get_or_create not provided with defaults

Reported by: jtauber Assigned to: gwilson
Milestone: 1.0 Component: Core framework
Version: SVN Keywords:
Cc: em@kth.se Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 1 Patch needs improvement: 1

Description

If get_or_create is given arguments that would necessitate a create but the necessary defaults are not provided, the exception thrown is DoesNotExist: Blah matching query does not exist.

It would be more useful if the error message explained the possibility that create failed because of missing defaults.

Attachments

6970.patch (0.6 kB) - added by deadwisdom on 07/01/08 17:00:00.
Integrity!

Change History

05/02/08 14:58:20 changed by Huvet

  • cc set to em@kth.se.
  • needs_better_patch changed.
  • needs_tests changed.
  • needs_docs changed.

I had a similar problem with get_or_create that took most of today to find (I'm a Django newbie). Seems as if get_or_create throws DoesNotExist? errors if you get database problems creating too. I had a Foreign key contraint that failed, but that error was somehow hidden by get_or_create. Really annoying! (Tip to anyone experiencing something similar: don't use get_or_create, use the two methods separately instead).

05/16/08 08:07:31 changed by bailey@akamai.com

specifically in django.db.models.query, line 228

if a value passed in defaults to get_or_create would cause a null constraint violation, handled like this:

except IntegrityError, e:

return self.get(**kwargs), False

guaranteed to reraise a DoesNotExist?, which is not helpful in the context of get_or_create

07/01/08 16:57:01 changed by deadwisdom

  • has_patch set to 1.

Yeah, this is horridly annoying. At some point, staring at the screen, I actually began to think that my Python interpreter was messed up. It seemed to not be catching an exception that it explicitly says to catch.

Anyhow, here's the patch to make some sense.

07/01/08 17:00:00 changed by deadwisdom

  • attachment 6970.patch added.

Integrity!

08/01/08 14:47:14 changed by shawnr

  • owner changed from nobody to shawnr.

08/01/08 15:51:29 changed by shawnr

  • owner deleted.
  • stage changed from Unreviewed to Ready for checkin.

I've tested this out at the DC Sprint and it seems to be working like a charm. Much nicer error message!

08/15/08 12:16:51 changed by mtredinnick

  • milestone set to 1.0.

Better just to catch the generic DoesNotExist error right the end (saves a couple of variable resolutions) and there's no need to capture e2 (it's never used). They can be fixed at checkin. Other than that, this is fine and does make things clearer.

08/15/08 12:17:56 changed by mtredinnick

  • needs_better_patch set to 1.
  • component changed from Uncategorized to Core framework.
  • needs_tests set to 1.
  • stage changed from Ready for checkin to Accepted.

On second thoughts, this should have tests so that we don't inadvertently break it again in the future.

08/15/08 17:08:20 changed by gwilson

  • owner set to gwilson.
  • status changed from new to assigned.

08/15/08 18:29:55 changed by gwilson

  • status changed from assigned to closed.
  • resolution set to fixed.

(In [8398]) Fixed #6970 -- Raise the original IntegrityError when all required fields aren't specified in get_or_create instead of a misleading DoesNotExist error, thanks deadwisdom.


Add/Change #6970 (misleading error message if get_or_create not provided with defaults)




Change Properties
Action