Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#22029 closed Cleanup/optimization (fixed)

documentation: new best place to put the signal handler registration

Reported by: Val Neekman Owned by: nobody
Component: Documentation Version: master
Severity: Normal Keywords:
Cc: vneekman@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The new best place for registering the signal handlers are in the ready() of the AppConfig.

This way, the signals properly emit with the server starts and also via the django-admin shell.

The documentation would benefit from an update indicating the new feature of Django 1.7.

The following section may require the update.
https://docs.djangoproject.com/en/dev/ref/signals/#django.db.models.signals.pre_migrate
https://docs.djangoproject.com/en/dev/ref/signals/#django.db.models.signals.post_migrate

This:
"Any handlers that listen to this signal need to be written in a particular place: a management module in one of your INSTALLED_APPS. If handlers are registered anywhere else they may not be loaded by migrate."
Could be changed to:
"Any handlers that listen to this signal need to be written in a particular place: the AppConfig.ready() within the apps module in one of your INSTALLED_APPS. If handlers are registered anywhere else they may not be loaded by migrate."

Attachments (1)

22029.diff (3.5 KB) - added by Tim Graham 3 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 3 years ago by Val Neekman

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

Also:

This:
"Sent by the migrate command before it starts to install an application."
Could be changed to:
"Sent by the migrate command before it starts to install an application that has a models module."

comment:2 Changed 3 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

Changed 3 years ago by Tim Graham

Attachment: 22029.diff added

comment:3 Changed 3 years ago by Tim Graham

Has patch: set

I wonder if we need to mention this as something specific to these signals though? Can we assume all signals will now be registered in AppConfig.ready()?

comment:4 Changed 3 years ago by Aymeric Augustin

I didn't check the commit logs, but the wording sounds like this is a warning against registering signals in urls.py (for instance). I don't think these signals are a special case any more.

comment:5 Changed 3 years ago by Val Neekman

The current docs says that you "need to" register these signals in the management module or else.

This is not the case anymore ... AppConfig.ready() is a better place, right Aymeric?

Also, circular dependencies are no longer the issue if these signals are registering in AppConfig.ready() and yes, no need to register in urls.py are more.

In short: "need to" is no longer true.

comment:6 Changed 3 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In 5233b366935dcc81e47546921ab4eb5007c83719:

Fixed #22029 -- Removed obsolete advice on registering migrate/syncdb signal handlers.

All signals should now be registered in AppConfig.ready().

Thanks un33k for the report.

comment:7 Changed 3 years ago by Tim Graham <timograham@…>

In 994274ea8315282484bc24aa403b2fd12b5bdeec:

[1.7.x] Fixed #22029 -- Removed obsolete advice on registering migrate/syncdb signal handlers.

All signals should now be registered in AppConfig.ready().

Thanks un33k for the report.

Backport of 5233b36693 from master

Note: See TracTickets for help on using tickets.
Back to Top