search

rails / activejob

Declare job classes that can be run by a variety of queueing backends
743 stars 47 forks source link

[feature] Initializer generator #94

Closed seuros closed 10 years ago

seuros commented 10 years ago

cc @rafaelfranca @cristianbica

DouweM commented 10 years ago

LGTM.

Because this will be part of Rails, configuring this in their config/application.rb or config/environments/*.rb rather than an initializer may be more appropriate, though. Should we already add this as Rails.application.config.active_job, or should we wait with that until AJ is actually part of Rails?

seuros commented 10 years ago

It is recommended to use the same adapter for all environments.
I was thinking about placing an inline adapter for the dev and test, but then any enqueued_at or enqueued_in job will not work.

DouweM commented 10 years ago

I think someone mentioned a test adapter that extends inline to just executes immediately on enqueue_at, doesn't sound too bad to me.

Doesn't change the fact that config/application.rb is more appropriate than an initializer though :)

seuros commented 10 years ago

I have no objection about config/application.rb, i still prefer the initializer way. if @dhh, @rafaelfranca, @guilleiguaran prefer the former, i can change this PR.

seuros commented 10 years ago

btw with config/application.rb, we will have write more code in the generator to avoid appending the config everytime the generator is run.

seuros commented 10 years ago

If you see rails master, you will see that assets stuff were extracted to assets.rb.

DouweM commented 10 years ago

Yeah, I agree that for now the initializer is fine, I'm simply pointing out that we should support the Rails.application.config style as well at some point, and config.active_job.queue_adapter will probably be in application.rb by default when this ships in Rails.

And I didn't know that about assets.rb! That changes this a bit, although that still seems to use Rails.application.config.

seuros commented 10 years ago

Probably because it already part of rails , see @guilleiguaran's response about similar issue

dhh commented 10 years ago

I like to keep application.rb as clean as possible, so not a fan of stuffing it in there.

But I also don't think we need this generator. We'll be integrating ActiveJob with Rails fully very shortly. So it'll just be in there by default.

seuros commented 10 years ago

It could be useful for people using it with rails 4.1 for example.