Allow unique args to receive the job hash

[daniel-g]

My project uses the Apartment gem in order to switch between schemas in the database. Since at the job creation we had the unique args as [item_id, 'schema name'] and at the execution we always had [item_id, 'public'], the jobs were always rejected. This code allows the unique_args method and proc to receive the current job hash in order to have more information about it at any point of the execution.

[mhenrixon]

Hi @daniel-g,

Thanks for the contribution! I am curious to understand your problem a little better because if this code gets merged it might potentially be a breaking change.

I'm not that familiar with the apartment gem so I'd like to understand the problem a little better since I was anyway thinking of using it in a project.

The item in this case is the sidekiq job hash and in my opinion it has no worth being sent to the unique args. Are you using some other sidekiq middleware that sets some values you need at this point? If so it might be better to use some other method of making the two middleware compatible. I'd rather extend the gem with say a configuration option to enable some check for whatever it is that you need to make it work for you.

[daniel-g]

Hi @mhenrixon, first of, thanks for the kind response, I'm very happy to contribute.

Basically the apartment gem switches between schemas in the database giving you the current schema in the method Apartment::Tenant.current, and influitive/apartment-sidekiq adds a key in the job hash called 'apartment' so that you don't have to add the tenant name every time you want to perform a job async.

After I issued this PR, I started trying moving around Sidekiq middlwares, since I wasn't aware of how they worked and I could actually figured it out: I did config.server_middleware.prepend(Apartment::Sidekiq::Middleware::Server) and that did the trick at a first glance, which in simple terms let me get the right tenant when scheduling the job and at job execution:

Apartment::Tenant.switch do |tenant| # apartment-sidekiq middleware
  ... other middlewares
  def unique_args(args) # unique jobs middleware
    # we do this because arguments are the same for all tenants
    [*args, Apartment::Tenant.current] # this worked fine now that the middleware was before unique jobs
  end
end

BUT, I found this doesn't play well with retries. If the job fails, the retry will try to get the unique arguments again, and this time Apartment::Tenant.current will always return 'public', so I had to get back to my old solution, I don't know how retries work and why the unique jobs gem doesn't get the unique arguments already set at the scheduling time:

Apartment::Tenant.switch do |tenant|
  ... other middlewares
  def unique_args(args, job) # unique jobs middleware, my fork
    # this method is being called at the scheduling time, execution, and retries.
    [*args, job.fetch("apartment", Apartment::Tenant.current)]
  end
end

I wonder why unique_args is being called on every step of the job instead of getting the key from the job if it's already set. I hope to have been clear in my explanation 👍 . Thanks for your amazing job!!

[mhenrixon]

Well, the order of the middleware is not something that I can easily fix. That said it might be possible to add the middleware last for this gem.

All things considered, I'd love to better support the apartment gem since I will most likely need that soon anyway. I think it makes absolute sense to provide uniqueness extension for the apartment gem however so that we can absolutely solve in a less intrusive way.

As for why the uniqueargs are called twice. This is a weird one, there were some issues in the past where someone said it was the only way to get things to work. It should be idempotent though so I definitely need to rethink this.

Let me do some thinking on this. I'll get back to sometime this week with more of a plan.

[mhenrixon]

Hi @daniel-g in the future you don't really need to do anything special. The unique middleware will be added after apartment (if the constant Apartment::Sidekiq::Middleware::Client/Server is defined). I will also ALWAYS consider the apartment for uniqueness. It will never again be a problem and it was a very simple change so I am very happy about it.

Checkout #540 if you are interested in how ;)

[daniel-g]

That's awesome !!! The code looks good, I just wonder if that can be done also in 6.x since we can't easily use the new version of sideqik.Thanks for the great effort :)

Do you think the problem with running unique_args many times was the ||= thing on your PR?

[mhenrixon]

Released as v6.0.25 @daniel-g