#22422 closed Cleanup/optimization (fixed)
RegistryNotReadyException in django.apps.registry.Apps.check_ready()
Reported by: | Vidir Valberg Gudmundsson | Owned by: | nobody |
---|---|---|---|
Component: | Documentation | Version: | 1.7-beta-1 |
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
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.
Change History (9)
comment:1 by , 11 years ago
comment:2 by , 11 years ago
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 :)
comment:3 by , 11 years ago
Component: | Core (Other) → Documentation |
---|---|
Triage Stage: | Unreviewed → Accepted |
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.
comment:4 by , 11 years ago
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).
comment:5 by , 11 years ago
Has patch: | set |
---|
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.
comment:6 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
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.