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

Ensure command details are populated for Cluster clients #1026

Merged
merged 1 commit into from Sep 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 6 additions & 7 deletions lib/redis/cluster/command_loader.rb
Expand Up @@ -10,22 +10,21 @@ module CommandLoader
module_function

def load(nodes)
details = {}

nodes.each do |node|
details = fetch_command_details(node)
details.empty? ? next : break
begin
return fetch_command_details(node)
rescue CannotConnectError, ConnectionError, CommandError
next # can retry on another node
end
end

details
raise CannotConnectError, 'Redis client could not connect to any cluster nodes'
end

def fetch_command_details(node)
node.call(%i[command]).map do |reply|
[reply[0], { arity: reply[1], flags: reply[2], first: reply[3], last: reply[4], step: reply[5] }]
end.to_h
rescue CannotConnectError, ConnectionError, CommandError
{} # can retry on another node
end

private_class_method :fetch_command_details
Expand Down
15 changes: 13 additions & 2 deletions test/cluster_client_key_hash_tags_test.rb
Expand Up @@ -6,8 +6,8 @@
class TestClusterClientKeyHashTags < Minitest::Test
include Helper::Cluster

def build_described_class
option = Redis::Cluster::Option.new(cluster: ['redis://127.0.0.1:7000'])
def build_described_class(urls = ['redis://127.0.0.1:7000'])
option = Redis::Cluster::Option.new(cluster: urls)
node = Redis::Cluster::Node.new(option.per_node_key)
details = Redis::Cluster::CommandLoader.load(node)
Redis::Cluster::Command.new(details)
Expand Down Expand Up @@ -85,4 +85,15 @@ def test_whether_the_command_effect_is_readonly_or_not
assert_equal false, described_class.should_send_to_slave?([:info])
end
end

def test_cannot_build_details_from_bad_urls
assert_raises(Redis::CannotConnectError) do
build_described_class(['redis://127.0.0.1:7006'])
end
end

def test_builds_details_from_a_mix_of_good_and_bad_urls
described_class = build_described_class(['redis://127.0.0.1:7006', 'redis://127.0.0.1:7000'])
assert_equal 'dogs:1', described_class.extract_first_key(%w[get dogs:1])
end
end