[Django] #32978: Provide more informative error message on loaddata/dumpdata --format yaml when PyYAML not installed
Django
when PyYAML not installed
-------------------------------------+-------------------------------------
Reporter: Brad | Owner: nobody
Type: | Status: new
Uncategorized |
Component: Core | Version: 3.2
(Management commands) | Keywords:
Severity: Normal | serializers,yaml,dumpdata,loaddata
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
While the docs at
https://docs.djangoproject.com/en/3.2/topics/serialization/#serialization-
formats do mention that PyYAML is required for the yaml serializer,
`./manage.py dumpdata|loaddata --format yaml`, provides quite a sparse
error message, "Unable to serialize database: module yaml not found" (from
here:
https://github.com/django/django/blob/fbb1984046ae00bdf0b894a6b63294395da1cce8/django/core/management/commands/dumpdata.py#L245)
if PyYAML is not installed.
This could be especially confusing for new Django developers who are not
aware that `yaml` is pointing to the PyYAML project on PyPI and is not a
part of Django. Since yaml is the only one of the four serializers
requiring an add-on third party package, it may be helpful to provide a
more verbose error in this case.
--
Ticket URL: <https://code.djangoproject.com/ticket/32978>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
when PyYAML not installed
-------------------------------------+-------------------------------------
Reporter: Brad | Owner: nobody
Component: Core (Management | Version: 3.2
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
serializers,yaml,dumpdata,loaddata | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> While the docs at
> https://docs.djangoproject.com/en/3.2/topics/serialization
> /#serialization-formats do mention that PyYAML is required for the yaml
> serializer, `./manage.py dumpdata|loaddata --format yaml`, provides quite
> a sparse error message, "Unable to serialize database: module yaml not
> found" (from here:
> https://github.com/django/django/blob/fbb1984046ae00bdf0b894a6b63294395da1cce8/django/core/management/commands/dumpdata.py#L245)
> if PyYAML is not installed.
>
> This could be especially confusing for new Django developers who are not
> aware that `yaml` is pointing to the PyYAML project on PyPI and is not a
> part of Django. Since yaml is the only one of the four serializers
> requiring an add-on third party package, it may be helpful to provide a
> more verbose error in this case.
New description:
While the docs at
https://docs.djangoproject.com/en/3.2/topics/serialization/#serialization-
formats do mention that PyYAML is required for the yaml serializer,
`./manage.py dumpdata|loaddata --format yaml`, provides quite a sparse
error message, "Unable to serialize database: module yaml not found" (from
here:
https://github.com/django/django/blob/fbb1984046ae00bdf0b894a6b63294395da1cce8/django/core/management/commands/dumpdata.py#L245)
if PyYAML is not installed.
This could be especially confusing for new Django developers who are not
aware that `yaml` is pointing to the PyYAML project on PyPI and is not a
part of Django. Since yaml is the only one of the four serializers
requiring an add-on third party package, it may be helpful to provide a
more verbose error in this case.
For instance, something like. "Serializing to and from YAML requires the
PyYAML package. Install PyYAML to use the YAML format."
--
--
Ticket URL: <https://code.djangoproject.com/ticket/32978#comment:1>
Django
when PyYAML not installed
-------------------------------------+-------------------------------------
Reporter: Brad | Owner: nobody
Type: | Status: new
Component: Core (Management | Version: 3.2
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
serializers,yaml,dumpdata,loaddata | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* type: Uncategorized => Cleanup/optimization
--
Ticket URL: <https://code.djangoproject.com/ticket/32978#comment:2>
Django
when PyYAML not installed
-------------------------------------+-------------------------------------
Reporter: Brad | Owner: nobody
Type: | Status: new
Component: Core | Version: 3.2
(Serialization) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
serializers,yaml,dumpdata,loaddata |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):
* component: Core (Management commands) => Core (Serialization)
* easy: 0 => 1
* stage: Unreviewed => Accepted
Comment:
Agreed, we can raise an `ImproperlyConfigured` error, e.g.
{{{
try:
import yaml
except ImportError as e:
raise ImproperlyConfigured(
'Error loading yaml module. Did you install PyYAML?'
) from e
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32978#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Brad | Owner: nobody
Type: | Status: new
Component: Core | Version: 3.2
(Serialization) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
serializers,yaml,dumpdata,loaddata |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
--
Ticket URL: <https://code.djangoproject.com/ticket/32978#comment:4>
Django
-------------------------------------+-------------------------------------
Type: | Krishna Jha
Cleanup/optimization | Status: assigned
Component: Core | Version: 3.2
(Serialization) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
serializers,yaml,dumpdata,loaddata |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* owner: nobody => Bal Krishna Jha
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/32978#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Brad | Owner: Bal
Type: | Krishna Jha
Cleanup/optimization | Status: assigned
Component: Core | Version: 3.2
(Serialization) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
serializers,yaml,dumpdata,loaddata |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Brad):
Interestingly, I found a comment in `django/core/serializers/pyyaml.py`:
`# Requires PyYaml (https://pyyaml.org/), but that's checked for in
__init__."`
However, I'm not seeing any such check in
`django/core/serializers/__init__.py`. (I expected a HAS_PYYAML or
something like that.)
There are also some tests in `tests/serializers/test_yaml.py` that test
specifically for
`YAML_IMPORT_ERROR_MESSAGE = r'No module named yaml'`
So those will need to be revised.
--
Ticket URL: <https://code.djangoproject.com/ticket/32978#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Brad | Owner: Bal
Type: | Krishna Jha
Cleanup/optimization | Status: assigned
Component: Core | Version: 3.2
(Serialization) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
serializers,yaml,dumpdata,loaddata |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Brad):
Some interesting background:
- https://github.com/django/django/pull/1582/files
- https://code.djangoproject.com/ticket/12756
--
Ticket URL: <https://code.djangoproject.com/ticket/32978#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Brad | Owner: Bal
Type: | Krishna Jha
Cleanup/optimization | Status: assigned
Component: Core | Version: 3.2
(Serialization) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
serializers,yaml,dumpdata,loaddata |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Brad):
Having read the background from Ticket 12756, the comment from
core/serializers/pyyaml.py about the checking being done in `__init__.py`
makes more sense now. The check that's referring to is here:
https://github.com/django/django/blob/main/django/core/serializers/__init__.py#L70
So again, the import error is caught correctly, but in this ticket I'm
proposing adding a friendlier message in the special case of yaml/PyYAML.
--
Ticket URL: <https://code.djangoproject.com/ticket/32978#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Brad | Owner: Bal
Type: | Krishna Jha
Cleanup/optimization | Status: assigned
Component: Core | Version: 3.2
(Serialization) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
serializers,yaml,dumpdata,loaddata |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
PR: https://github.com/django/django/pull/14729
Old:
```sh
$ ./manage.py dumpdata --format yaml --pks 1 users.User
CommandError: Unable to serialize database: No module named 'yaml'
```
New:
```sh
$ ./manage.py dumpdata --format yaml --pks 1 users.User
CommandError: Unable to serialize database: Error loading yaml module. Did
you install PyYAML?
```
--
Ticket URL: <https://code.djangoproject.com/ticket/32978#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Brad | Owner: Bal
Type: | Krishna Jha
Cleanup/optimization | Status: assigned
Component: Core | Version: 3.2
(Serialization) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
serializers,yaml,dumpdata,loaddata |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Brad):
I do realize this is arguably an addition of kludgy code, especially the
test_yaml.py changes, in exchange for a small improvement in the error
message, so my feelings won't be hurt either way on this one.
--
Ticket URL: <https://code.djangoproject.com/ticket/32978#comment:10>
Django
-------------------------------------+-------------------------------------
Type: | Status: assigned
Cleanup/optimization |
Component: Core | Version: 3.2
(Serialization) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
serializers,yaml,dumpdata,loaddata |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* owner: Bal Krishna Jha => Brad
--
Ticket URL: <https://code.djangoproject.com/ticket/32978#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: Brad | Owner: Brad
Cleanup/optimization |
Component: Core | Version: 3.2
(Serialization) |
Keywords: | Triage Stage: Accepted
serializers,yaml,dumpdata,loaddata |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):
* status: assigned => closed
* resolution: => wontfix
Comment:
Replying to [comment:10 Brad]:
> I do realize this is arguably an addition of kludgy code, especially the
test_yaml.py changes, in exchange for a small improvement in the error
message, so my feelings won't be hurt either way on this one.
Thanks for all your efforts! I agree that it's not worth changing.
--
Ticket URL: <https://code.djangoproject.com/ticket/32978#comment:12>