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

Does not work the default ActionMailer queue introduced in Rails 4.2 #244

Closed
myobie opened this issue Jan 15, 2015 · 15 comments
Closed

Does not work the default ActionMailer queue introduced in Rails 4.2 #244

myobie opened this issue Jan 15, 2015 · 15 comments

Comments

@myobie
Copy link

myobie commented Jan 15, 2015

The default queue where emails are added is "mailers" and it should be assumed that queue is being worked by default. delayed_job does this.

This can be worked around with QUEUES=mailers rake jobs:work for now.

@jipiboily
Copy link
Contributor

Got it. So, we could either make it listen by default. Not sure of the side impact of this, probably not much. That said, it's not going to make 3.1 as it would be a breaking change.

@senny: thoughts on this? Do you see a reason not to change this behavior? I personally would not change it, but don't have a strong opinion. If it makes it easier for some people to get started, maybe it makes sense.

@senny
Copy link
Contributor

senny commented Jan 16, 2015

@jipiboily I wouldn't change it to work both "default" and "mailers" if nothing else is specified. That seems very arbitrary. I see the following possible solutions:

  1. Have the default bin/rake qc:work work all queues unless specified otherwise
  2. In the queue classic railtie configure ActionMailer to use the default queue instead of the mailers queue.

@jipiboily
Copy link
Contributor

Yeah, sorry for the confusion, I had in mind all queues by default, or stick with the current...as the only two decent options.

I would lean toward the first option you gave, or the status quo.

@senny
Copy link
Contributor

senny commented Jan 16, 2015

I'm fine with option 1.)

@jipiboily
Copy link
Contributor

I don't have time to work on this but if @myobie or anyone else want to send a PR, that would be awesome!

@myobie
Copy link
Author

myobie commented Jan 16, 2015

I would love to help. After looking more into it I don't see an easy way to work all queues yet, so if you have any suggestions of where to look let me know.

@senny
Copy link
Contributor

senny commented Jan 18, 2015

@myobie that's correct. Currently there's no q_name wildcard for the worker to get it to work all queues.

@myobie
Copy link
Author

myobie commented Jan 19, 2015

OK, then I think it's best to add a feature to accept QUEUE=* and then just make that the return value of default_queue. Does that sound good? I'll try to get a PR for that this week.

@senny
Copy link
Contributor

senny commented Jan 19, 2015

@myobie would that really solve the problem? Imagine default_queue is set to "default". How would QUEUE=* pickup on the mailer queue?

@myobie
Copy link
Author

myobie commented Jan 19, 2015

I meant that default_queue would be "*".

@senny
Copy link
Contributor

senny commented Jan 19, 2015

@myobie sounds good 😊

@myobie
Copy link
Author

myobie commented Jan 23, 2015

The way the triggers work makes it difficult to listen to all queues, since the locks are per queue_name. I thought about doing something like this:

@@ -75,6 +75,7 @@ $$ LANGUAGE plpgsql;
 -- queue_classic_notify function and trigger
 create function queue_classic_notify() returns trigger as $$ begin
   perform pg_notify(new.q_name, '');
+  perform pg_notify('*', new.q_name);
   return null;
 end $$ language plpgsql;

but the lock_head function still is by queue_name. Listening to all queues could require a complete change of things and I'm not sure that's a good idea.

So I wanted to ask your opinion of what might be best? Is it worth going down this road or should this really just be a documentation problem?

@senny
Copy link
Contributor

senny commented Jan 24, 2015

On Rails we have rails/rails#18587 open. I think it could be reasonable to change the default Action Mailer queue to default in our QC Railtie. It should be done in such a fashion that the user can still overwrite that configuration option in either config/application.rb or config/environments/*.rb.

@jipiboily thoughts?

@jipiboily
Copy link
Contributor

I agree with @senny. I think changing the default via our railtie but making it possible to overwrite + document this in the README would be perfect!

@myobie
Copy link
Author

myobie commented Aug 16, 2018

I'm closing this since it's so old.

@myobie myobie closed this as completed Aug 16, 2018
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

No branches or pull requests

3 participants