sidekiq-unique-jobs icon indicating copy to clipboard operation
sidekiq-unique-jobs copied to clipboard

ActiveJob support

Open sunny opened this issue 4 years ago • 10 comments

Since the Sidekiq documentation has been updated to indicate that:

As of Sidekiq 6.0.1 you can use sidekiq_options with your Rails 6.0.1+ ActiveJobs.

Could ActiveJob support be reconsidered for sidekiq-unique-jobs?

(Related to #221 #238.)

sunny avatar Apr 04 '20 08:04 sunny

If all the sidekiq options are supported then nothing will have to be done for it to work. The problem with ActiveJob used to be the sidekiq middleware and customizing anything on a per-job basis.

I hope this doesn't come off as offensive (definitely not my intention); I still don't understand why anyone would choose to use ActiveJob when having access to Sidekiq. It just doesn't make sense to me. I mean, for some things it is required (like ActionMailbox) but it can still be used for that. For everything else, why chose perform_later over perfom_async? ActiveJob doesn't provide anything to my knowledge that Sidekiq doesn't do better?

mhenrixon avatar Apr 04 '20 13:04 mhenrixon

Hi @mhenrixon! Thanks for the clarification.

I still don't understand why anyone would choose to use ActiveJob when having access to Sidekiq

You're absolutely right to question using ActiveJob. I have been happily using Sidekiq without ActiveJob for years, and don't intend to switch away from using Sidekiq as a backend!

One of the benefits that I find is the fact that ActiveJob uses Rails’ GlobalIDs. It simplifies workers by being able to give them ActiveRecord instances instead of having to give ids (or even class names) that you need to fetch inside your worker.

There are other benefits that come with it simply being the convention:

  • Documentation, guides and teaching resources point to ActiveJob.
  • Not having to change your jobs if your application started using ActiveJob.
  • Addons and libraries that rely on ActiveJob.

For example I am thinking about adding support for async jobs in Actor and I think it would make more sense to write something for ActiveJob instead of having to implement it for Sidekiq, DelayedJob, Resque, etc.

sunny avatar Apr 05 '20 17:04 sunny

Thank you for sharing! I really appreciate the answer and then it makes way more sense why people are using it.

Given the support of sidekiq options work on a per job/worker I’d do everything I can to support it. That was hands down the root issue where we tried to globally handle every workers unique scenario in a single place.

I’ll have a proper read up on active job next week. Clearly missed a few things :)

mhenrixon avatar Apr 05 '20 17:04 mhenrixon

If all the sidekiq options are supported then nothing will have to be done for it to work.

@mhenrixon Does that mean the gem could now work with ActiveJob nicely without any modifications? I tried but it does not seem to be the case.

class DummyJob < ActiveJob::Base
  sidekiq_options lock: :until_executed
end

still allows duplicate jobs in the queue.

My environment includes Rails 6.0.3 & Sidekiq 6.0.7

Note: sidekiq_options works with Rails 6.0.2 and above only, not 6.0.1 as stated in this comment https://github.com/mperham/sidekiq/issues/4281#issuecomment-554664409

longnd avatar Jul 02 '20 03:07 longnd

@longnd since I don't know ActiveJob I can't fully help you here. The assumption is that it would work but if ActiveJob is passing it's global id to the arguments it might be that you need to filter some argument for uniqueness to be considered.

mhenrixon avatar Jul 02 '20 07:07 mhenrixon

Here is another approach: activejob-uniqueness

sharshenov avatar Jul 05 '20 21:07 sharshenov

@sharshenov Looks quite good :+1:

tonobo avatar Jul 06 '20 04:07 tonobo

I don't think global ids should interfere with uniqueness, since they're just a string. They get serialized in the arguments as something like:

{"_aj_globalid"=>"gid://my_app/User/13987607"}

mockdeep avatar Mar 15 '21 22:03 mockdeep

Here is another approach: activejob-uniqueness

This is by far the better solution. Seems there is a lot of pushback to including ActiveJob in this gem so picking up an AJ specific gem really does make a lot of sense here.

danielricecodes avatar Feb 16 '23 19:02 danielricecodes

Not to slight this project at all, but as I was also looking at ActiveJob support and crossed by this thread -- Sidekiq Throttled allows you to support both ActiveJob and Sidekiq::Job at the same time ~(I am swapping from activejob-uniqueness myself, as Throttled supports both limiting duplicated jobs as well as general concurrency controls)~

Slight update: but while it will support both job classes, but does not support the use case for uniqueness in the sense of dropping duplicates that both this gem and aj-uniqueness seem to support.

So sorry for the misdirection, I learned the hard way 🥲

jeffbax avatar Mar 27 '24 14:03 jeffbax