Opened 11 years ago

Closed 9 years ago

#19820 closed Cleanup/optimization (fixed)

Make loaddata error messages less cryptic

Reported by: bigfudge Owned by: nobody
Component: Core (Serialization) Version: dev
Severity: Normal Keywords:
Cc: reames@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

At present, error messages when running loaddata don't include any clue about the part of the serialised data which caused the problem.

In the Deserialiser function (in core.serializers.json.py) there is a try/except block on line 40 to 47 which could be amended to aid the user:

def Deserializer(stream_or_string, **options):
    """
    Deserialize a stream or string of JSON data.
    """
    if isinstance(stream_or_string, basestring):
        stream = StringIO(stream_or_string)
    else:
        stream = stream_or_string
    try:
        for obj in PythonDeserializer(simplejson.load(stream), **options):
            yield obj
    except GeneratorExit:
        raise
    except Exception, e:
        # Map to deserializer error
        raise DeserializationError(e)

If raise DeserializationError(e) were changed to raise DeserializationError(e, obj) then the function would return a tuple containing the current error message (which isn't that informative) and the object which causes the error.

This has been a big issue for me recently in migrating a pilot installation which used sqlite over to postgres. There were a number of data integrity issues which using postgres highlighted, but it was only by applying this change that it became possible to identify the records which were causing the problem.

Change History (19)

comment:1 by Claude Paroz, 11 years ago

Better error messages are always welcome. However, you mention integrity issues, while the fix you suggest is at deserialization stage. Maybe some actual example could help here?

comment:2 by Carl Meyer, 11 years ago

Resolution: needsinfo
Status: newclosed

Closing needsinfo pending clarification of whether the better error messages are really needed at deserialization or database-insert, and an example clearly demonstrating the problem that the proposed fix would ameliorate.

comment:3 by Richard Eames, 10 years ago

If there is, for example, an initialiser for auth.Permission with a content_type that does not exist, the DeserializationError is raised:

django.core.serializers.base.DeserializationError: Problem installing fixture 'testapp/initial_data/testing_data.json': ContentType matching query does not exist.

This is a problem with big files, since it doesn't tell you where in the file the error occurred. (Reproduced on 1.7c3)

comment:4 by Richard Eames, 10 years ago

Cc: reames@… added

comment:6 by Claude Paroz, 10 years ago

Has patch: set
Needs tests: set
Resolution: needsinfo
Status: closednew

comment:7 by Richard Eames, 10 years ago

I've add a simple patch which tests that an error message is raised, and that the message contains the offending model name, it's PK, and what the offending field's value is.

Let me know if there's anything else I can add.

comment:8 by Tim Graham, 10 years ago

Needs tests: unset

comment:9 by Tim Graham, 10 years ago

Triage Stage: UnreviewedAccepted

comment:10 by Tim Graham, 10 years ago

Patch needs improvement: set

I left comments for improvement on the PR. Please uncheck "Patch needs improvement" when you update it, thanks.

comment:11 by Richard Eames, 10 years ago

Patch needs improvement: unset

New PR https://github.com/django/django/pull/3223

I left some questions I need some feedback on.

comment:12 by Tim Graham, 9 years ago

Patch needs improvement: set

Patch needs a rebase.

comment:13 by Richard Eames, 9 years ago

Rebased, and responded to question.

comment:14 by Tim Graham, 9 years ago

Patch needs improvement: unset

Don't forget to uncheck "Patch needs improvement" so the ticket appears in the review queue.

comment:15 by Tim Graham, 9 years ago

Patch needs improvement: set

Tests need to fixed on Python 3.

comment:16 by Richard Eames, 9 years ago

Patch needs improvement: unset

Fixed python 3 error: it was due to python3 "fixing" variables escaping generator expressions.

comment:17 by Tim Graham, 9 years ago

Patch needs improvement: set

Left some more comments.

comment:18 by Richard Eames, 9 years ago

Patch needs improvement: unset

Rebased and updated patch.

comment:19 by Tim Graham <timograham@…>, 9 years ago

Resolution: fixed
Status: newclosed

In 727e40c8:

Fixed #19820 -- Added more helpful error messages to Python deserializer.

Note: See TracTickets for help on using tickets.
Back to Top