#22688 closed Bug (fixed)
Apps.set_installed_apps() re-executes AppConfig.ready() methods
| Reported by: | Aymeric Augustin | Owned by: | nobody |
|---|---|---|---|
| Component: | Core (Other) | Version: | 1.7-beta-2 |
| Severity: | Release blocker | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
The current implementation of set_installed_apps() calls populate(), which is going to execute ready() methods of applications.
We should:
- either add a flag to the app registry to track whether ready() has been executed yet,
- or document that ready() must be idempotent.
Change History (6)
comment:1 by , 11 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:2 by , 11 years ago
This is interesting but very tied to set_installed_apps. I was thinking of a flag on the AppConfig instance. It would be simple and easy to reset if needed.
Also I think there's a use case for reexecuting ready() whenever INSTALLED_APPS changes: apps that set up things based on INSTALLED_APPS.
comment:3 by , 11 years ago
Then the documentation approach:
diff --git a/docs/ref/applications.txt b/docs/ref/applications.txt
index 829e273..f68ea38 100644
--- a/docs/ref/applications.txt
+++ b/docs/ref/applications.txt
@@ -238,6 +238,15 @@ Methods
separate from the production settings, ``manage.py test`` would still
execute some queries against your **production** database!
+ .. note::
+
+ In the usual initialization process, the ``ready`` method is only called
+ once by Django. But in some corner cases, particularly in tests which
+ are fiddling with installed applications, ``ready`` might be called more
+ than once. In that case, either write idempotents methods, or put a flag
+ on your ``AppConfig`` classes to prevent re-running code which should
+ be executed exactly one time.
+
.. _namespace package:
Namespace packages as apps (Python 3.3+)
I purposefully mentioned AppConfig classes, not instances, because I think that populate is recreating new AppConfig instances.
comment:4 by , 11 years ago
| Has patch: | set |
|---|---|
| Triage Stage: | Accepted → Ready for checkin |
Excellent, can you commit that?
It keeps the code simple and at worst we can adjust the behavior if it proves to be a problem in practice.
comment:5 by , 11 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
A possible fix:
diff --git a/django/apps/registry.py b/django/apps/registry.py index 41095bd..6c89a69 100644 --- a/django/apps/registry.py +++ b/django/apps/registry.py @@ -109,7 +109,14 @@ class Apps(object): self.ready = True for app_config in self.get_app_configs(): - app_config.ready() + if not self.has_been_loaded(app_config.label): + app_config.ready() + + def has_been_loaded(self, app_label): + for stored in self.stored_app_configs: + if app_label in stored: + return True + return False def check_ready(self): """