search

brandonhilkert / sucker_punch

Sucker Punch is a Ruby asynchronous processing library using concurrent-ruby, heavily influenced by Sidekiq and girl_friday.
MIT License
2.65k stars 114 forks source link

Error when sending async email using sucker_punch as ActiveJob queue adapter #156

Closed drueck closed 8 years ago

drueck commented 8 years ago

I'm using :sucker_punch as my active_job.queue_adapter, and when I tried updating to 2.0.0.beta1, I got this error when trying to call #deliver_later on one of my ActionMailer emails:

NoMethodError (undefined method `async' for #
<ActiveJob::QueueAdapters::SuckerPunchAdapter::JobWrapper:0x007fbca8888530>)

Is there any change in how I need to configure SuckerPunch to work with ActiveJob and ActionMailer? Or has the SuckerPunchAdapter just not been updated for 2.0 yet?

Let me know if you'd like any more info. Thanks!

brandonhilkert commented 8 years ago

Rails hasn't been updated to work with 2.0.0. That's on my list once it's gets out of beta.

drueck commented 8 years ago

Oh, I see. I didn't realize the sucker punch adapter was actually part of Rails and not this project. I guess I should have dug into the code a bit first. Thanks for the quick reply!

drueck commented 8 years ago

Side note: It seems kind of unfortunate that all the ActiveJob adapters live in the Rails codebase instead of in their own projects or separate adapter projects. Seems like a shame for people to have to wait for a new release of Rails to use a newer version of an ActiveJob compatible provider. Interesting...

brandonhilkert commented 8 years ago

It's definitely not ideal, but it is their way of making sure the whole thing doesn't get out of control, which as a user of Rails, I can appreciate.

drueck commented 8 years ago

Curated list of approved quality adapters, I guess. Yeah, that makes sense at that level. I guess that'll just give me one more reason to update to Rails 5 soon. ;) Thanks for your work on this! Much appreciated!

jdantonio commented 8 years ago

If you use the "Backward Compatibility" feature mentioned in the README, require 'sucker_punch/async_syntax', that may allow you to use ActiveJob with the v2 beta.

drueck commented 8 years ago

@jdantonio Good point! I tried that and my ActiveJob based tasks (emails and background image processing) are now working with 2.0.0.beta1. It might make sense to add a comment to the README about using that backward compatibility feature when using it with ActiveJob pre-Rails 5?

openface commented 8 years ago

Just to confirm this... The new sucker_punch should work fine with an older version of Rails (ie 4.0.X) if it's used directly and not implemented through ActiveJob, correct?

brandonhilkert commented 8 years ago

@openface Yes.

brandonhilkert commented 8 years ago

@drueck Added note here

drueck commented 8 years ago

Perfect. Great clarification for other users looking to upgrade.

brandonhilkert commented 8 years ago

@drueck This is handled in Rails now - https://github.com/rails/rails/pull/23284

drueck commented 8 years ago

Nice! I checked out the commit. Looks like celluloid was still being required. Someone just commented asking about it. Perhaps it was just a small oversight. Anyway, glad to see when I update to Rails 5 I'll be good to go!