[Django] #22422: RegistryNotReadyException in django.apps.registry.Apps.check_ready()
Django
--------------------------------------+------------------------
Reporter: valberg | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Other) | Version: 1.7-beta-1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------
Instead of using RuntimeError in django.apps.registry.Apps.check_ready(),
we should use a django defined exception (possible inheriting from
RuntimeError). This way, we can easily catch the exception at the level of
occurence, and reraise it with an informative message.
For instance, using django.utils.translation.ugettext in models.py raises
a RuntimeError. This does not aid in explaining where the error occurs.
Here we could raise RegistryNotReadyException('Use
django.utils.translation.ugettext_lazy instead of ugettext').
In the spirit of the new checks framework, let us make it easier for
developers to recognize errors and try to inform them how they might be
able to fix them. For instance importing ugettext instead of ugettext_lazy
in models.py raises an unexplained RuntimeError when running the 'check'
command.
--
Ticket URL: <https://code.djangoproject.com/ticket/22422>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: valberg | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version:
Component: Core (Other) | 1.7-beta-1
Severity: Normal | Resolution:
Keywords: | Triage Stage:
Has patch: 0 | Unreviewed
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by aaugustin):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
I'm not sure I understand your proposal. Do you suggest to wrap every call
to the app registry with a try / except and reraise errors with a specific
error message?
First, while Django has had a very bad habit to try to be helpful with
exceptions by catching them and reraising a different exception, this
hasn't delivered the expected results. In practice it tends to hide the
real cause make it more complicated to understand errors. Getting rid of
this pattern is even a GSoC project topic! We have tracebacks to figure
out where errors come from.
(Once we drop support for Python 2, this argument won't apply anymore. But
that isn't for today.)
Second, there's over 150 such occurrences in the code base, and I find it
unrealistic to hope that every future maintainer will properly wrap every
call to the app cache like you suggest.
Using a custom exception for exceptions thrown by the app registry may
still make it slightly easier to search for related information in the
documentation or on the Internet.
--
Ticket URL: <https://code.djangoproject.com/ticket/22422#comment:1>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization | Version:
Component: Core (Other) | 1.7-beta-1
Severity: Normal | Resolution:
Keywords: | Triage Stage:
Has patch: 0 | Unreviewed
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by valberg):
It might well be that's just not that good at deciphering tracebacks :)
The idea was to make it easier to figure out what is actually going wrong,
as you write in the end of your comment.
I can definitely see the point that it would add more complexity for
future maintenance.
Maybe it's more down to a documentation issue. I solved my problem reading
https://docs.djangoproject.com/en/dev/releases/1.7/#start-up-sequence.
Including something about these RuntimeErrors in
https://docs.djangoproject.com/en/1.7/ref/applications/ would definitely
be an acceptable solution seen from my point of view :)
--
Ticket URL: <https://code.djangoproject.com/ticket/22422#comment:2>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* component: Core (Other) => Documentation
* stage: Unreviewed => Accepted
Comment:
I looked at this again today and I don't think it's very useful to change
`raise RuntimeError("App registry isn't ready yet.")` to `raise
AppRegistryNotReady("App registry isn't ready yet.")`. Putting the error
message into Google takes you straight to the 1.7 release notes, which are
the best resource on this problem at this time.
The application loading process isn't documented. It would be worth saying
at least a few words in `ref/applications.txt` and copying the
troubleshooting advice there.
--
Ticket URL: <https://code.djangoproject.com/ticket/22422#comment:3>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization | Version:
Component: Documentation | 1.7-beta-1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by valberg):
I've just made a pull request
(https://github.com/django/django/pull/2540). Basically just copying from
the release notes to the reference page. I'm not knowledgable enough about
the loading process to write it down.
I know it's a bit redundant to have this in both the release notes and the
reference page. But it just seems right to be able to rely on the
documentation without the release notes (even though they technically are
a part of the documentation).
--
Ticket URL: <https://code.djangoproject.com/ticket/22422#comment:4>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization | Version:
Component: Documentation | 1.7-beta-1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* has_patch: 0 => 1
Comment:
I think I would prefer linking the release notes to the text in the
documentation so as we make additions and/or corrections we don't need to
update two places. The PR should be against master, then we backport from
there.
--
Ticket URL: <https://code.djangoproject.com/ticket/22422#comment:5>
Django
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization | Version:
Component: Documentation | 1.7-beta-1
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"deb561bbe2dbacac32d65272beab3a7708e2c31d"]:
{{{
#!CommitTicketReference repository=""
revision="deb561bbe2dbacac32d65272beab3a7708e2c31d"
Fixed #22422 -- Moved information about the application loading process to
refs/applications.txt.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22422#comment:6>
Django
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization | Version:
Component: Documentation | 1.7-beta-1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"bde1bc66724db34dcde598f064448da1f2a8a810"]:
{{{
#!CommitTicketReference repository=""
revision="bde1bc66724db34dcde598f064448da1f2a8a810"
[1.7.x] Fixed #22422 -- Moved information about the application loading
process to refs/applications.txt.
Backport of deb561bbe2 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22422#comment:7>
Django
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization | Version:
Component: Documentation | 1.7-beta-1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"0315f01087d75f68654f62883a9f36a0f9f71676"]:
{{{
#!CommitTicketReference repository=""
revision="0315f01087d75f68654f62883a9f36a0f9f71676"
Fixed a confusing heading in applications docs.
Refs #22422.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22422#comment:8>
Django
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization | Version:
Component: Documentation | 1.7-beta-1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"788fce4c917eee27b10f38f7eb0b75cd2af3ec43"]:
{{{
#!CommitTicketReference repository=""
revision="788fce4c917eee27b10f38f7eb0b75cd2af3ec43"
[1.7.x] Fixed a confusing heading in applications docs.
Refs #22422.
Backport of 0315f01 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22422#comment:9>