feat: add status endpoint pro vsphere provider#38
feat: add status endpoint pro vsphere provider#38ondrej-hosak wants to merge 1 commit intoAVGTechnologies:masterfrom
Conversation
| running_machines: ::Compute.where(provider_name: provider_name, state: 'running').count, | ||
| errored_machines: ::Compute.where(provider_name: provider_name, state: 'errored').count, | ||
| created_machines: ::Compute.where(provider_name: provider_name, state: 'created').count, | ||
| queued_machines: ::Compute.where(provider_name: provider_name, state: 'queued').count |
There was a problem hiding this comment.
I suggest to pass come possibility to specify what states should be returned; these hardcoded: running, errored, created and queued might be default option. The other thing: v_sphere specific measure is only max_running_machines: the rest is the same for all possible providers :) what about this?
There was a problem hiding this comment.
We can think about this improvement if this route starts to run 'slowly' but for now i think it's ok to return all information.
| module Endpoints | ||
| # class responsible for provider statuses | ||
| class Status < Base | ||
| get '/v_sphere' do |
There was a problem hiding this comment.
Just note, not merge blocker. When you look at the code in the .to_json block, you can clearly see it is parametrised.
We could have internal functions get_status or get_status_json, which would take provider_name as parameter.
Then, this route would be simplified to something like:
get '/v_sphere' do
get_status(provider_name).to_json
Benefit now? Not so big. Benefit for future? Definitely, once new providers pop in.
| run LabManager::App::Endpoints::Compute.new | ||
| end | ||
|
|
||
| map '/status' do |
There was a problem hiding this comment.
This cannot give me rest (pun intended).
I am not sure about singular in route name, what do you think?
| it 'returns errored machines count' do | ||
| response_json = perform_request | ||
|
|
||
| expect(response_json['errored_machines']).to eq 2 |
There was a problem hiding this comment.
Maybe you add expects for specific machines as well (like "one" and "two" present)
3 participants
This is temporary modification for service and its provider monitoring.