Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add ensure_consumer kwarg to before/after_declare_queue to control whether a consumer should be created on a RabbitMQ, then disable consumer thread creation upon message delivery #670

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jenstroeger
Copy link
Contributor

Following the discussion Issue with RabbitMQ Broker: sending a message to the queue creates a consumer thread locally, this draft illustrates the proposed change.

@Bogdanp I also noticed that there’s no

self.emit_before("declare_delay_queue", delayed_name)

callback when the DQ is declared — is that intentional?

…rol whether a consumer should be created on a RabbitMQ, then disable consumer thread creation upon message delivery
@Bogdanp
Copy link
Owner

Bogdanp commented Dec 24, 2024

Thanks, but, unfortunately, this would be a breaking change for anyone already implementing one of these hooks, so I don't think this can work (and I don't think the problem warrants complicating the code to add arity checks to work around the breakage). Until we can come up with a better solution, I think the way to go is that I proposed in that discussion. A potential alternative would be to allow some level of worker configuration -- eg. to allow a user to extend the worker middleware in some way, but I haven't had time to think about the implications of allowing that.

@jenstroeger
Copy link
Contributor Author

Thanks, but, unfortunately, this would be a breaking change for anyone already implementing one of these hooks, so I don't think this can work (and I don't think the problem warrants complicating the code to add arity checks to work around the breakage).

That’s why I used kwargs with defaults so that existing code continues to work.

Until we can come up with a better solution, I think the way to go is that I proposed in that discussion.

I agree, though I’m not enthusiastic about either one approach… 😉

A potential alternative would be to allow some level of worker configuration -- eg. to allow a user to extend the worker middleware in some way, but I haven't had time to think about the implications of allowing that.

And that leads back to exposing maybe too much of Dramatiq internals. Hmm. Maybe let’s sit on it for a few days?

Also, what about the other question above, the missing invocation of the before_declare_delay_queue callback?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants