Skip to content
This repository was archived by the owner on Apr 16, 2019. It is now read-only.

Conversation

snopoke
Copy link
Contributor

@snopoke snopoke commented Oct 19, 2015

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing this means the signal only gets fired once (for this app) instead of for every app.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious if you think we need to worry about this:

The ready() method may be executed more than once during testing, so you may want to guard your signals from duplication, especially if you’re planning to send them within tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so but could be good practice to do it anyway.

On 19 October 2015 at 19:46, Ethan Soergel notifications@github.com wrote:

In fluff/app_config.py
#105 (comment):

@@ -0,0 +1,10 @@
+from django.apps import AppConfig
+from django.db.models.signals import post_migrate
+from fluff.signals import catch_signal
+
+
+class FluffAppConfig(AppConfig):

  • name = 'fluff'
  • def ready(self):
  •    post_migrate.connect(catch_signal, sender=self)
    

Curious if you think we need to worry about this
https://docs.djangoproject.com/en/1.8/topics/signals/#connecting-receiver-functions
:

The ready() method may be executed more than once during testing, so you
may want to guard your signals from duplication
https://docs.djangoproject.com/en/1.8/topics/signals/#preventing-duplicate-signals,
especially if you’re planning to send them within tests.


Reply to this email directly or view it on GitHub
https://github.com/dimagi/fluff/pull/105/files#r42402361.

Simon Kelly
Senior Engineer | Dimagi South Africa

czue added a commit that referenced this pull request Oct 19, 2015
@czue czue merged commit 5464bf1 into master Oct 19, 2015
@czue czue deleted the migrate branch October 19, 2015 09:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants