Opened 2 years ago

Closed 4 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 2 years ago by claudep

  • 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 2 years ago by carljm

  • Resolution set to needsinfo
  • Status changed from new to closed

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 11 months ago by Naddiseo

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 11 months ago by Naddiseo

  • Cc reames@… added

comment:6 Changed 11 months ago by claudep

  • Has patch set
  • Needs tests set
  • Resolution needsinfo deleted
  • Status changed from closed to new

comment:7 Changed 11 months ago by Naddiseo

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 11 months ago by timgraham

  • Needs tests unset

comment:9 Changed 11 months ago by timgraham

  • Triage Stage changed from Unreviewed to Accepted

comment:10 Changed 11 months ago by timgraham

  • 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 11 months ago by Naddiseo

  • 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 7 months ago by timgraham

  • Patch needs improvement set

Patch needs a rebase.

comment:13 Changed 7 months ago by Naddiseo

Rebased, and responded to question.

comment:14 Changed 7 months ago by timgraham

  • Patch needs improvement unset

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

comment:15 Changed 5 months ago by timgraham

  • Patch needs improvement set

Tests need to fixed on Python 3.

comment:16 Changed 5 months ago by Naddiseo

  • Patch needs improvement unset

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

comment:17 Changed 4 months ago by timgraham

  • Patch needs improvement set

Left some more comments.

comment:18 Changed 4 months ago by Naddiseo

  • Patch needs improvement unset

Rebased and updated patch.

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

  • Resolution set to fixed
  • Status changed from new to closed

In 727e40c8:

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

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