mhenrixon / sidekiq-unique-jobs

Prevents duplicate Sidekiq jobs
MIT License
1.43k stars 273 forks source link

ActiveJob support #487

Open sunny opened 4 years ago

sunny commented 4 years ago

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.)

mhenrixon commented 4 years ago

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?

sunny commented 4 years ago

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:

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.

mhenrixon commented 4 years ago

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 :)

longnd commented 4 years ago

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

mhenrixon commented 4 years ago

@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.

sharshenov commented 4 years ago

Here is another approach: activejob-uniqueness

tonobo commented 4 years ago

@sharshenov Looks quite good :+1:

mockdeep commented 3 years ago

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"}
danielricecodes commented 1 year ago

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.

jeffbax commented 3 months ago

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 🥲