[Django] #19159: loaddata reports DeserializationError instead of MemoryError
Django
-------------------------------+--------------------
Reporter: django@… | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.4
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
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)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19159>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: django@… | Owner: nobody
Type: Bug | Status: new
(Serialization) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by anonymous):
* needs_better_patch: => 0
* component: Uncategorized => Core (Serialization)
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/19159#comment:1>
Django
Reporter: django@… | Owner: nobody
Type: Bug | Status: new
(Serialization) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by lrekucki):
In Python 3 this could be written as {{{raise DeserializationError from
e}}}, but I don't see a better equivalent in Python 2.
--
Ticket URL: <https://code.djangoproject.com/ticket/19159#comment:2>
Django
Reporter: django@… | Owner: nobody
Type: Bug | Status: new
(Serialization) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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()`.
--
Ticket URL: <https://code.djangoproject.com/ticket/19159#comment:3>
Django
Reporter: django@… | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/19159#comment:4>
Django
Reporter: django@… | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by anonymous):
Possibly fixed by #5423.
--
Ticket URL: <https://code.djangoproject.com/ticket/19159#comment:5>
Django
Reporter: django@… | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/19159#comment:6>
Django
Reporter: django@… | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by berkerpeksag):
`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.
--
Ticket URL: <https://code.djangoproject.com/ticket/19159#comment:7>
Django
Reporter: django@… | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jacob Walls):
* has_patch: 0 => 1
* version: 1.4 => dev
Comment:
[https://github.com/django/django/pull/14270 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/19159#comment:8>
Django
Reporter: django@… | Owner: nobody
Type: Bug | Status: closed
Component: Core (Serialization) | Version: dev
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* status: new => closed
* resolution: => invalid
Comment:
`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'
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19159#comment:9>
Django
Reporter: django@… | Owner: nobody
Type: Bug | Status: closed
Component: Core (Serialization) | Version: dev
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/19159#comment:10>