Skip to content

Commit

Permalink
Fix the current ops issues
Browse files Browse the repository at this point in the history
  • Loading branch information
pilhuhn committed Jun 24, 2016
1 parent 825afa7 commit 4f66c74
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 9 deletions.
20 changes: 18 additions & 2 deletions lib/hawkular/operations/operations_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,30 @@ def to_msg_hash
# @example
# Hawkular::Operations::OperationsClient.new(credentials: {username: 'jdoe', password: 'password'})
def initialize(args)
ep = args[:entrypoint]

unless ep.nil?

This comment has been minimized.

Copy link
@abonas

abonas Jun 26, 2016

I wonder why the client doesn't accept uri type in the first place, because right now the callers are doing construct uri.to_s, and then here it is transformed back to be uri.

uri = URI.parse ep
args[:host] ||= "#{uri.host}:#{uri.port}"

This comment has been minimized.

Copy link
@abonas

abonas Jun 26, 2016

this is a bit confusing to me. the name of the parameter is host, but potentially it can get both host and port as its value according to the above.

end

args[:host] ||= 'localhost:8080'

This comment has been minimized.

Copy link
@abonas

abonas Jun 26, 2016

iiic this still needs resolution (and removal of the default)

This comment has been minimized.

Copy link
@pilhuhn

pilhuhn Jun 27, 2016

Author Owner

I've opened hawkular#81 for this

args[:credentials] ||= {}
args[:options] ||= {}
args[:wait_time] ||= 0.5

This comment has been minimized.

Copy link
@abonas

abonas Jun 26, 2016

what is the time unit here for wait_time?

This comment has been minimized.

Copy link
@pilhuhn

pilhuhn Jun 27, 2016

Author Owner

This is not my code :) - I expect seconds.

super(args[:host], args[:credentials], args[:options])
# note: if we start using the secured WS, change the protocol to wss://
url = "ws://#{entrypoint}/hawkular/command-gateway/ui/ws"
@ws = Simple.connect url do |client|
url = "ws://#{args[:host]}/hawkular/command-gateway/ui/ws"
ws_options = {}
creds = args[:credentials]
incoming_opts = args[:options]

This comment has been minimized.

Copy link
@abonas

abonas Jun 26, 2016

why not use below directly args[:options][:tenant] ? (or extract the tenant fully here rather that half extract here and then some more extract below)

This comment has been minimized.

Copy link
@pilhuhn

pilhuhn Jun 27, 2016

Author Owner

That is a left-over from testing against a server that in fact did not have a user set up (but where I was not aware of the fact)

ws_options[:headers] = { 'Authorization' => 'Basic ' +
["#{creds[:username]}:#{creds[:password]}"].pack('m').delete("\r\n"),
'Hawkular-Tenant' => incoming_opts[:tenant],
'Accept' => 'application/json'
}

@ws = Simple.connect url, ws_options do |client|
client.on(:message, once: true) do |msg|
parsed_message = msg.data.to_msg_hash
puts parsed_message if ENV['HAWKULARCLIENT_LOG_RESPONSE']

This comment has been minimized.

Copy link
@abonas

abonas Jun 26, 2016

where does this puts go btw? (I know, I know, it's not part of the commit)
it's kind of equivalent of System.out.println in Java...

This comment has been minimized.

Copy link
@pilhuhn

pilhuhn Jun 27, 2016

Author Owner

Stdout - see the README for the use of that env variable.

Expand Down
37 changes: 31 additions & 6 deletions spec/integration/operations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,47 @@ module Hawkular::Operations::RSpec
end

it 'should be established' do
WebSocketVCR.configure do |c|
c.hook_uris = [HOST]
end
# WebSocketVCR.configure do |c|

This comment has been minimized.

Copy link
@abonas

abonas Jun 26, 2016

why is this commented out?

This comment has been minimized.

Copy link
@pilhuhn

pilhuhn Jun 27, 2016

Author Owner

To be able to test the code at all. I was not able to turn off the VCR by any other means.

# c.hook_uris = [HOST]
# end

WebSocketVCR.record(example, self) do
# WebSocketVCR.record(example, self) do
client = OperationsClient.new(host: HOST,
wait_time: WebSocketVCR.live? ? 1.5 : 0,
credentials: {
username: 'jdoe',
password: 'password'
},
options: {
tenant: 'hawkular'
})
ws = client.ws
expect(ws).not_to be nil
expect(ws.open?).to be true
end
end
# end
end

it 'should be established2' do
# WebSocketVCR.configure do |c|
# c.hook_uris = [HOST]
# end

# WebSocketVCR.record(example, self) do
ep = URI::HTTP.build(:host => 'acme.org', :port => 42).to_s

This comment has been minimized.

Copy link
@pilhuhn

pilhuhn Jun 27, 2016

Author Owner

With this 'bogus' host/port, the connection will wait for 60s until it runs in a timeout. See also hawkular#80

client = OperationsClient.new(entrypoint: ep,
wait_time: WebSocketVCR.live? ? 1.5 : 0,
credentials: {
username: 'jdoe',
password: 'password'
},
options: {
tenant: 'hawkular'
})
ws = client.ws
expect(ws).not_to be nil
expect(ws.open?).to be true
# end
end
end

describe 'Operation/Operation', :websocket, vcr: { decode_compressed_response: true } do
Expand Down
2 changes: 1 addition & 1 deletion spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ def record(prefix, bindings, explicit_cassette_name, example: nil)
if ENV['VCR_OFF'] == '1'
VCR.eject_cassette
VCR.turn_off!(ignore_cassettes: true)
WebSocketVCR.turn_off!
WebSocketVCR.turn_off! # TODO this does not work the impl is empty
WebMock.allow_net_connect!
puts 'VCR is turned off!'
end
Expand Down

0 comments on commit 4f66c74

Please sign in to comment.