Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Passing the timeout as a positional argument is deprecated, it should be passed as a keyword argument #5488

Closed
ldlgds2 opened this issue Aug 23, 2022 · 11 comments

Comments

@ldlgds2
Copy link

ldlgds2 commented Aug 23, 2022

Ruby version: 3.0.4
Rails version: 6.1.6.1
Sidekiq / Pro / Enterprise version(s): 6.5.5

Passing the timeout as a positional argument is deprecated, it should be passed as a keyword argument:
redis.brpop("queue:default", timeout: 2)(called from: /usr/local/bundle/gems/sidekiq-6.5.5/lib/sidekiq/fetch.rb:49:in 'block in retrieve_work')
Redis#sadd will always return an Integer in Redis 5.0.0. Use Redis#sadd? instead.(called from: /usr/local/bundle/gems/sidekiq-6.5.5/lib/sidekiq/launcher.rb:168:in 'block (2 levels) in ❤')

Gem redis 4.8.0 seems to be issue, downgrading to 4.7.1 fixed it temporarily.

@mperham
Copy link
Collaborator

mperham commented Aug 23, 2022

Already fixed on main, leaving open so others will see it and not open duplicates. #5484 #5485 #5486

@tisba
Copy link

tisba commented Aug 25, 2022

Not sure I'm overlooking something. But 09dacfe#diff-0c49098f0471d2ffa897441bd05c7d24c04ce480e66cb534b194a4bb15e0b201R168 is probably not fixing the warning

Redis#sadd will always return an Integer in Redis 5.0.0. Use Redis#sadd? instead.(called from: /usr/local/bundle/gems/sidekiq-6.5.5/lib/sidekiq/launcher.rb:168:in 'block (2 levels) in ❤')

@schorsch
Copy link

@tisba seems like all args where converted to the required array syntax by v4.8 of the redis gem https://github.com/redis/redis-rb/blob/v4.8.0/lib/redis/commands/sets.rb#L22

@rsl
Copy link

rsl commented Aug 29, 2022

this is eating up logging like mad for us. is there a way to put out a gem release with this fix soon? would be awesome. our logging keeps filling up.

@mperham
Copy link
Collaborator

mperham commented Aug 29, 2022

@rsl No one is forcing you to use Redis 4.8.0. You can downgrade.

@rsl
Copy link

rsl commented Aug 29, 2022

wasn't trying to be rude. just asking a question. not sure if there's a hold up on releasing a new gem version or something?

@ryanb
Copy link

ryanb commented Aug 29, 2022

Looks like Sidekiq 6.5.6 is out which includes this fix.

@aaricpittman
Copy link

Just to confirm, 6.5.6 should fix the issue? Or do I also need to downgrade the redis gem?

Because I'm still seeing the error and we are on 6.5.6.

@mperham
Copy link
Collaborator

mperham commented Aug 30, 2022

@aaricpittman It should. If you are still seeing the issue, it's possibly due to a 3rd party plugin.

@coderberry
Copy link

I am seeing these error come from the sidekiq-throttled gem (issue link).

@virusman
Copy link

@coderberry Thanks, I found the issue in my project to be caused by an outdated version of sidekiq-limit_fetch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants