Bogdanp / django_dramatiq

A Django app that integrates with Dramatiq.
https://dramatiq.io
Other
332 stars 77 forks source link

Initialization should only be done once #133

Closed huubbouma closed 1 year ago

huubbouma commented 1 year ago

The ready method in DjangoDramatiqConfig is called multiple times durin the startup of Django. This was introduced in the fix for issue #26 The broker is a global variable so it's overwritten a second time the method is called (in phase 3) The newly created instance of the broker will not have the already registered actors from phase 2.

huubbouma commented 1 year ago

I also noticed the extra call to the ready() method when using django_periodiq This addon has a DjangoPeriodiqConfig class which inherits from DjangoDramatiqConfig and therefore also ends up in making the ready call in django_periodiq twice more (during phase 2 and 3). This is also fixed by the pull requst

amureki commented 1 year ago

Hey @huubbouma !

Thank you for the bug report and provided PR. I want to try to clarify the issue you are experiencing, so we can come up with a final solution.

May I get a bit more information about your setup? This will help with debugging and may help with adding a regression test to cover your case.

  1. Was 0.10.0 and 0.11.0 working fine with your configuration?
  2. Where do you register your actors, where do they get imported?
  3. How do you know that a new broker instance is not seeing registered actors?
  4. Do you have any custom configuration set in ready() method?

Best, Rust

huubbouma commented 1 year ago

Hi Rust,

  • Was 0.10.0 and 0.11.0 working fine with your configuration?

To be honest, I haven't tested it

  • Where do you register your actors, where do they get imported?

Some are imported from the AppConfig, so during phase 1 I think, which is probably part of the problem here.

  • How do you know that a new broker instance is not seeing registered actors?

I put a print() statement in the dramatiq broker class

  • Do you have any custom configuration set in ready() method?

Yes, in one of my apps I register receivers to django signals which in the end causes this issue because it loads the @dramatic.actor decorator

PR #137 fixes this issue for me

amureki commented 1 year ago

@huubbouma thanks for all the input! I just released 0.11.2 with this fix.

Best, Rust