[Compass-ci] Re: [PATCH v1 compass-ci 3/4] providers/libvirt: request scheduler to consume job

+# register hostname mac queues +# request scheduler +class Consumer < Base

• attr_reader :info
• def initialize(hostname, queues)
• @info = {}
• @info['hostname'] = hostname
• @info['queues'] = queues

@info = { 'hostname' => hostname, 'queues' => queues }

• end
• def close
• del_host_info
• end

this function is useless, directly invoke del_host_info if enough

• def request_job
• get_mac_from_hostname(@info['hostname'])

you invoked the get_mac_from_hostname only here, if you use values of @info, donot need to add the parameter in () just use @info['hostname'] in the function

• config_scheduler
• set_host_info
• host_exists
• url = "http://#%7B@sched_host%7D:#%7B@sched_port%7D/boot.libvirt/mac/#%7B@info%5B%2..."
• @logger.info("Request URL: #{url}")
• res = %x(curl #{url})
• if res.empty?

•  @logger.error('Can not connect scheduler')
  

•  raise 'Can not connect scheduler'
  

• end
• JSON.parse(res)
• end
• private
• def config_scheduler
• names = Set.new %w[

•  SCHED_HOST
  

•  SCHED_PORT
  

• ]
• defaults = relevant_defaults(names)
• @sched_host = defaults['SCHED_HOST'] || '172.17.0.1'
• @sched_port = defaults['SCHED_PORT'] || 3000
• @logger.info("SCHED_HOST: #{@sched_host}")
• @logger.info("SCHED_PORT: #{@sched_port}")
• end

I think you can set the 'names =' and 'defaults =' out of the class and set two constant

puts @sched_host and @sched_port to initialize part

• def get_mac_from_hostname(hostname)
• cmd = %(echo #{hostname} | md5sum | sed "s/^\(..\)\(..\)\(..\)\(..\)\(..\).*$/0a-\1-\2-\3-\4-\5/")
• @info['mac'] = %x(#{cmd}).chomp
• @logger.info("Mac address: #{@info['mac']}")
• end
• def set_host_info
• system "curl -X PUT \

•       'http://#{@sched_host}:#{@sched_port}/set_host_mac?hostname=#{@info['hostname']}&mac=#{@info['mac']}'"
  

• system "curl -X PUT \

•       'http://#{@sched_host}:#{@sched_port}/set_host2queues?host=#{@info['hostname']}&queues=#{@info['queues']}'"
  

better avoid use command if possible. you can reference github.com/rest-client/rest-client for some help

• end
• def del_host_info
• system "curl -X PUT 'http://#%7B@sched_host%7D:#%7B@sched_port%7D/del_host_mac?mac=#%7B@info%5B%2...'"
• system "curl -X PUT 'http://#%7B@sched_host%7D:#%7B@sched_port%7D/del_host2queues?host=#%7B@info%...'"

ditto

• end
• def host_exists
• @info['host'] = @info['hostname'].split('.')[0]

if @info['host'] is only used here, use 'host = ' is better.

Thanks, Luan Shengde

• host_file = "#{ENV['LKP_SRC']}/hosts/#{@info['host']}"
• return if FileTest.exists?(host_file)
• @logger.error("The file does't exist: #{host_file}")
• raise "The file does't exist: #{host_file}"
• end

+end

2.23.0