davydovanton / sidekiq-statistic

See statistic about your workers
MIT License
797 stars 84 forks source link

Fix Problem with Sidekiq 7 #189

Closed dougmrqs closed 1 year ago

dougmrqs commented 1 year ago

Due to Sidekiq 7 changes on its Redis gem dependency, the behavior changed a bit. Also, I'm trying to figure out if there is an actual need to use pipelined flow for all those operations.

My goal is to maintain compatibility between Sidekiq versions: While Sidekiq 6.x uses redis gem, Sidekiq 7.x uses redis-client which is more straightforward. Sidekiq 6.x used to return a Redis::Future object from pipeline operations, which had to be called with .value method to extract the length (return of the function) of it. Now the call for the list length is always returning an Integer.

I've wrapped some calls to be pipelined, leaving the lpush out, so it directly returns an Integer.

This work relates to Issue #188

dougmrqs commented 1 year ago

I've added a second commit for us to evaluate if it is really needed to have that pipeline.

dougmrqs commented 1 year ago

I think we could make good use of the PR #184, adding Sidekiq 6.x and 7.x to the pipeline to make sure everything integrates. Apparently now we don't have any CI implementations.

wenderjean commented 1 year ago

This looks good. Thanks for the contribution @dougmrqs