Opened 9 years ago

Closed 5 months ago

Last modified 5 months ago

#19159 closed Bug (invalid)

loaddata reports DeserializationError instead of MemoryError

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

Description

The command manage.py loaddata data.json errors with DeserializationError instead of MemoryError.

files:

../lib/python2.7/site-packages/django/core/serializers/json.py
/usr/lib/python2.7/json/__init__.py

relevant code is simplejson.load(stream) inside the try:

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)

Change History (10)

comment:1 Changed 9 years ago by anonymous

Component: UncategorizedCore (Serialization)

comment:2 Changed 9 years ago by Łukasz Rekucki

In Python 3 this could be written as raise DeserializationError from e, but I don't see a better equivalent in Python 2.

comment:3 Changed 9 years ago by anonymous

i think there's too much going on in the try statement:

        for obj in PythonDeserializer(simplejson.load(stream), **options):
            yield obj

the error arises from simplejson.load(stream) when reading a large json file from disk during fp.read().

comment:4 Changed 9 years ago by Aymeric Augustin

Triage Stage: UnreviewedAccepted

comment:5 Changed 9 years ago by anonymous

Possibly fixed by #5423.

comment:6 Changed 9 years ago by Claude Paroz

No, #5423 is fixing dumpdata, this ticket is about loaddata. The problem with loaddata is that it would be very difficult to incrementally read the file.

comment:7 Changed 7 years ago by Berker Peksag

json.Deserializer now uses six.reraise (see #18003). I think there are two questions that needs to be answered here:

  1. Should DeserializationError wrap MemoryError? If not, we need a patch to catch MemoryError explicitly.
  2. If yes, the ticket's title and description should be updated.

See also #12007.

comment:8 Changed 5 months ago by Jacob Walls

Has patch: set
Version: 1.4dev

PR on the assumption we don't want to wrap MemoryError. If we do want to wrap it, we're already chaining the exception with raise from so we can just close the ticket.

comment:9 Changed 5 months ago by Mariusz Felisiak

Resolution: invalid
Status: newclosed

MemoryError is not wrapped anymore because we no longer use StringIO or BytesIO, and MemoryError is raised by .read() (see 7d0f8831922535502569a5dda989dde339b4e491):

  File "django/django/core/serializers/json.py", line 65, in Deserializer
    stream_or_string = stream_or_string.read()
MemoryError: Problem installing fixture 'mydump.json'

comment:10 Changed 5 months ago by Claude Paroz

I'm not so sure that MemoryError is only triggered by read(), but let's see how it goes and if the issue happens again in the future.

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