Getting all the master nodes in an elasticsearch cluster

Question

I am trying to get all the master nodes in an elasticsearch cluster using this code. Can I reduce/simply this or optimize further?

#!/usr/bin/env ruby
require 'elasticsearch'
client_node = ARGV[0]
client = Elasticsearch::Client.new host: client_node
clients = client.nodes.info process: true
clients['nodes'].each do |node|
 node_id = node[0]
 puts clients['nodes'][node_id]['host'] if clients['nodes'][node_id]['attributes']['master']== "true"
end

Answer 1

1. Fix your indentation. Ruby uses 2 spaces of indentation.

2. Your naming is confusing. You have client_node, client and clients, which are all very different things (string, object, hash as far as I can tell). The only one that's well-named is client since that is indeed an instance of Client. But client_node you can skip entirely since you can just as well write ARGV.first (which is more Rubyesque than [0]) in the one place you need it. Or you can call it host since that what it is. And clients could be called client_info or something similar. Name it for what it is.

   Note: In the following I keep your names for simplicity's sake, but you should really reconsider the naming in your code.

3. You can iterate keys and values in loops:

   clients['nodes'].each do |name, info|
    puts info['host'] if info['attributes']['master'] == "true"
   end
   

4. Things will break if the info hash doesn't have an attributes key - your code is assuming it's there. A way to avoid this is to use Hash#fetch to absorb nil values:

   puts info['host'] if info.fetch('attributes', {})['master'] == 'true'
   

5. Since all you want is the master node, it'd be more expressive to find that node first - if it exists - and then do what you want with it:

   master_node = clients['nodes'].detect do |name, info|
    info.fetch('attributes', {})['master'] == 'true'
   end
   puts master_node['host'] if master_node
   

Beyond that, I feel confident the Elasticsearch library exposes a bunch of convenience methods you can use instead of accessing values with string keys. There may also be a way to fetch just the master node, instead of looping through stuff to find it. But I don't know the library, so it's just a guess.