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

Fix redis deprecation warnings #44

Merged
merged 2 commits into from
Jul 15, 2022

Conversation

lavaturtle
Copy link
Contributor

This fixes deprecation warnings, seen when running the specs in this gem or using it, that look like this:

Pipelining commands on a Redis instance is deprecated and will be removed in Redis 5.0.0.

redis.multi do
  redis.get("key")
end

should be replaced by

redis.multi do |pipeline|
  pipeline.get("key")
end

(called from /home/grey/code/sidekiq-grouping/lib/sidekiq/grouping/redis.rb:16:in `block in push_msg'}

Also: When I first tried to run the specs, I ran into an error that looked like rails/rails#43851, so this adds a require line that fixes the error.

@@ -1,3 +1,4 @@
require "active_support"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.

conn.rpush(ns(name), msg)
conn.sadd(unique_messages_key(name), msg) if remember_unique
conn.multi do |pipeline|
pipeline.sadd(ns('batches'), name)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@reinaris
Copy link

reinaris commented May 6, 2022

@gzigzigzeo is this worth merging? Or do we need to fix the hound comments?

@mrexox
Copy link
Contributor

mrexox commented Jul 14, 2022

@gzigzigzeo , what do you think about this PR? This change seems useful especially for the future versions of Redis.

@gzigzigzeo
Copy link
Owner

Thank you!

@gzigzigzeo gzigzigzeo merged commit 14aaf1c into gzigzigzeo:master Jul 15, 2022
@lavaturtle lavaturtle deleted the redis-deprecation-warnings branch July 15, 2022 13:47
@lavaturtle lavaturtle restored the redis-deprecation-warnings branch July 15, 2022 13:48
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

Successfully merging this pull request may close these issues.

5 participants