Opened 4 years ago

Closed 18 months ago

#19820 closed Cleanup/optimization (fixed)

Make loaddata error messages less cryptic

Reported by: bigfudge Owned by: nobody
Component: Core (Serialization) Version: master
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 Changed 4 years ago by Claude Paroz

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

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 Changed 4 years ago by Carl Meyer

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 Changed 2 years ago by Richard Eames

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 Changed 2 years ago by Richard Eames

Cc: reames@… added

comment:6 Changed 2 years ago by Claude Paroz

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

comment:7 Changed 2 years ago by Richard Eames

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 Changed 2 years ago by Tim Graham

Needs tests: unset

comment:9 Changed 2 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

comment:10 Changed 2 years ago by Tim Graham

Patch needs improvement: set

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

comment:11 Changed 2 years ago by Richard Eames

Patch needs improvement: unset

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

I left some questions I need some feedback on.

comment:12 Changed 21 months ago by Tim Graham

Patch needs improvement: set

Patch needs a rebase.

comment:13 Changed 21 months ago by Richard Eames

Rebased, and responded to question.

comment:14 Changed 21 months ago by Tim Graham

Patch needs improvement: unset

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

comment:15 Changed 19 months ago by Tim Graham

Patch needs improvement: set

Tests need to fixed on Python 3.

comment:16 Changed 19 months ago by Richard Eames

Patch needs improvement: unset

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

comment:17 Changed 18 months ago by Tim Graham

Patch needs improvement: set

Left some more comments.

comment:18 Changed 18 months ago by Richard Eames

Patch needs improvement: unset

Rebased and updated patch.

comment:19 Changed 18 months ago by Tim Graham <timograham@…>

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