Skip to content
This repository has been archived by the owner on Apr 27, 2022. It is now read-only.

Move show_console over to obmd #1034

Merged
merged 8 commits into from
Sep 5, 2018
Merged

Conversation

zenhack
Copy link
Contributor

@zenhack zenhack commented Aug 31, 2018

This is the last API call that actually uses the drivertized API support. A subsequent PR will delete the old functionality.

Still futzing with travis; I'm getting an error in the CLI tests that I can't reproduce locally.

@zenhack
Copy link
Contributor Author

zenhack commented Aug 31, 2018

Ok, tests are passing.

@zenhack zenhack requested a review from naved001 August 31, 2018 21:43
Copy link
Contributor

@naved001 naved001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.
Things we would need to update in the next PR
[ ] Remove all old code
[ ] Update the API docs
[ ] Upade the config files (sample, and dev-no-hardware).

for the next release of HIL we should also include the call to get power status.
so an API for
[ ] get power status API.
[ ] Make a new obmd release

@naved001
Copy link
Contributor

naved001 commented Sep 4, 2018

self.obmd_uri + '/' + path,

On a related node, how does this line of code even work in our tests. As far as I know we the url to enable the obm (which returns the token) is obmd_uri/node/<node-name>/token. But in that method we just do obmd_uri/path where path is just token in this case.

This is what I did locally to get it working:

         return requests.request(
             method,
-            self.obmd_uri + '/' + path,
+            self.obmd_uri + '/node/' + self.label + path,
             auth=('admin', self.obmd_admin_token),

But how are the integration tests passing then

@zenhack
Copy link
Contributor Author

zenhack commented Sep 4, 2018

The obmd_uri field in the model is the path to that node in the obmd api, not the base path to the obmd api. so it includes the '/node/' + self.label already. This is per the spec.

@naved001
Copy link
Contributor

naved001 commented Sep 4, 2018

is the path to that node in the obmd api

ah, I forgot about that bit. I did the registration wrong in that case.

Copy link
Contributor

@ianballou ianballou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

url = self.object_url('node', node, 'console')
response = self.httpClient.request('GET', url)
# we don't call check_response here because we want to return the
# raw byte stream, rather than converting it to json.
# raw byte stream, rather reading the whole thing in and converting
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than *

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.

@naved001 naved001 merged commit 83c8f6d into CCI-MOC:master Sep 5, 2018
@zenhack zenhack deleted the forward-console branch September 6, 2018 19:58
@zenhack zenhack mentioned this pull request Sep 6, 2018
3 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants