Bogdanp / django_dramatiq

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

dramatiq.set_broker(broker) in Phase 3 #26

Closed nanopony closed 5 years ago

nanopony commented 5 years ago

I suspect I found a case when django_dramatiq is too late to set global_broker configured from settings, and actor decorator gets default RabbitMq broker from get_broker()

Django app registry populate consists of three phases:

Hence, in my case the models.py imports a task from tasks.py. That happens on Phase 2, prior to the moment of ready() of DjangoDramatiqConfig is called.

I understand that broker initialization must be put into the ready() due to middleware that can require apps to be initialized first. However resolving this issue could improve quality of life a lot.

As for now, I guess local imports of task will do.

Kind regards, Serj

Bogdanp commented 5 years ago

I'm going to make it set up the broker before ready() is called as a fix. The way things are set up right now, this issue is baffling to new users. I figure it's better to make it harder to write middleware than normal code if we are forced to choose between the two.

sergioshev commented 5 years ago

Hi Bogdanp I'm facing the same issue. Any updates on this?

Bogdanp commented 5 years ago

@sergioshev I made the change in https://github.com/Bogdanp/django_dramatiq/commit/29646b09e47783221f6559b6177d87720884d9fc . What are you trying to do and what is the exact issue you're running into?

sergioshev commented 5 years ago

I came up here from https://github.com/Bogdanp/dramatiq/issues/168. I followed your example from django dramatiq in github. Defined a simple actor but after calling my tasks.py I'm getting and error.

When I run rundramatiq all workers are ok, and tasks module is detected. `$ python manage.py rundramatiq

[2019-05-16 14:20:55,475] [PID 8061] [Thread-2] [dramatiq.worker.ConsumerThread(default)] [ERROR] Received message for undefined actor 'poll_odds_sport_radar'. Moving it to the DLQ. Traceback (most recent call last): File "/home/odds-service/venv/lib/python3.5/site-packages/dramatiq/worker.py", line 314, in handle_message actor = self.broker.get_actor(message.actor_name) File "/home/odds-service/venv/lib/python3.5/site-packages/dramatiq/broker.py", line 214, in get_actor raise ActorNotFound(actor_name) from None dramatiq.errors.ActorNotFound: poll_odds_sport_radar

tasks.py is very simple

import dramatiq                                                                                                           
from pprint import pprint
from dramatiq.brokers.redis import RedisBroker

redis_broker = RedisBroker(host="localhost", port=6379)
dramatiq.set_broker(redis_broker)

@dramatiq.actor
def poll_odds_sport_radar():
    pprint("poller for sport radar should be implemented here")

if __name__ == "__main__":
    poll_odds_sport_radar.send()

Thank you Bogdan for your time.

CapedHero commented 5 years ago

Maybe the problem lies with the fact, that you called set_broker yourself?

Try your luck with the suggested DRAMATIQ_BROKER configuration in settings.py, see Installation instructions in the README of this repo.

sergioshev commented 5 years ago

Thank you for your response. I have settings.py exactly as described in the example. settings.py

...
DRAMATIQ_REDIS_URL = os.getenv("REDIS_URL", "redis://127.0.0.1:6379/0")

DRAMATIQ_BROKER = {
    "BROKER": "dramatiq.brokers.redis.RedisBroker",
    "OPTIONS": {
        "connection_pool":
            redis.ConnectionPool.from_url(DRAMATIQ_REDIS_URL),
    },
    "MIDDLEWARE": [
        "dramatiq.middleware.AgeLimit",
        "dramatiq.middleware.TimeLimit",
        "dramatiq.middleware.Retries",
        "django_dramatiq.middleware.AdminMiddleware",
        "django_dramatiq.middleware.DbConnectionsMiddleware",
    ]
}
...

But if I remove manual setting for RedisBroker from tesks.py, HOC actor is failing with this error

Traceback (most recent call last): File "tasks.py", line 9, in @dramatiq.actor File "/home/odds-service/venv/lib/python3.5/site-packages/dramatiq/actor.py", line 225, in actor return decorator(fn) File "/home/odds-service/venv/lib/python3.5/site-packages/dramatiq/actor.py", line 220, in decorator priority=priority, broker=broker, options=options, File "/home/odds-service/venv/lib/python3.5/site-packages/dramatiq/actor.py", line 52, in init self.broker.declare_actor(self) File "/home/odds-service/venv/lib/python3.5/site-packages/dramatiq/broker.py", line 174, in declare_actor self.declare_queue(actor.queue_name) File "/home/odds-service/venv/lib/python3.5/site-packages/dramatiq/brokers/rabbitmq.py", line 219, in declare_queue raise ConnectionClosed(e) from None dramatiq.errors.ConnectionClosed: AMQPConnectionError: (AMQPConnectorSocketConnectError: ConnectionRefusedError(111, 'Connection refused'),)

For some reason HOC is trying to use rabbitmq. BTW is a fresh/clean django setup (django/django-dramatiq) in Debian stable (stretch). After setting the broker to redis by hand, I see error as above (Actor not found). No clue that is wrong with my set up.

CapedHero commented 5 years ago

Ah, I think I got it! Everything is pretty clear once you follow the stack trace, step by step.

You are loading default settings. Why? Because you call the tasks via a self-running script (I see if __name__ == "__main__": there). Try calling the task from a fully functional Django app instead.

@Bogdanp BTW, I am not sure if having default settings is beneficial. When I was setting the project up, I was also very confused about the messages that I don't have an active connection to Rabbit (even though I have explicitly used Redis). I think that more direct exceptions would work better here, like:

DramatiqException: Hello clumsy! Dramatiq is not configured! Did you forget to set your broker?

DjangoDramatiqException: Hello clumsy! Django-Dramatiq is not configured! Did you call the task from a working Django application with a properly configured 'DRAMATIQ_BROKER' in the settings?
sergioshev commented 5 years ago

Thank you @CapedHero . I'll will try your suggestion.

Bogdanp commented 5 years ago

BTW, I am not sure if having default settings is beneficial. When I was setting the project up, I was also very confused about the messages that I don't have an active connection to Rabbit (even though I have explicitly used Redis). I think that more direct exceptions would work better here, like:

Yes, I'm inclined to agree. I'll consider this as a potential breaking change in 2.0.

@sergioshev I believe @CapedHero is right. The issue is that you're trying to enqueue the messages from a standalone python script.