citusdata / citusdata/activerecord-multi-tenant
Sidekiq Middleware registered in the wrong order with Batches when used with Sidekiq Pro
- Dominant language
- Ruby
- Stars
- 759
- Forks
- 113
- PR merge metrics
- No merged PRs in 30d
Description
When using `activerecord-multi-tenant` with the paid Sidekiq-Pro gem, sidekiq [batch callbacks](https://github.com/sidekiq/sidekiq/wiki/Batches#callback-details) do not have a tenant set by default, even if the original job and batch were enqueued inside a tenant block.
Given the following example job:
```ruby
class TestJob
include Sidekiq::Job
def perform
puts "perform"
puts MultiTenant.current_tenant
end
def callback(status, options)
puts "callback"
puts MultiTenant.current_tenant
end
end
```
and the following execution (assuming `1` is a valid tenant ID):
```ruby
MultiTenant.with(1) do
b = Sidekiq::Batch.new
b.on(:success, "TestJob#callback")
b.jobs { TestJob.perform_async }
end
```
You will find that while the tenant is set correctly in `perform`, it is not set in `callback`. This is because of a middleware ordering issue between the multitenant code and the sidekiq-pro batch code.
We were able to work around this issue by manually removing and re-ordering the middleware in our own initializer:
```ruby
# Fix middleware order so that Sidekiq Batch Callbacks are run with a tenant set
chain.remove(Sidekiq::Middleware::MultiTenant::Client)
chain.insert_before(Sidekiq::Batch::Client, Sidekiq::Middleware::MultiTenant::Client)
```
(and the same thing for the server middleware, and the client middleware on the server).
---
Given that this is a sidekiq-builtin feature, it would be nice if this gem could check for the presence of the Batch middleware and insert its middleware in the correct spot by default.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.