Code review comment for lp:~thumper/juju-core/debug-log-api-client

Revision history for this message
Tim Penhey (thumper) wrote :

Reviewers: mp+214875_code.launchpad.net,

Message:
Please take a look.

Description:
Add the debug-log client api

This involved a little more work than I expected.
The client connection now caches the x509 pool so it can
be passed along with the websocket connection for the debug
log call.

A key point of interest, is that if the remote api server
does not have a websocket listener on '/log', the dial config
just blocks and never returns, so we can't wait for an error
on that to determine that the remote api server doesn't support
the api yet. I thought it would be good to be able to ask
the api server what version it was running, so in future, if we
hit this again, we can at least have a version check. The mere
existance of the version call is enough for us to determine that
the remote server supports debug-log, so we don't care about
the actual value.

Initially I used a bufio scanner to read the first line of the
response, however that reads 4k into a buffer, which killed all
the tests. I now read a byte at a time to get the json encoded
error. It would probably be much better to use the websocket
methods to read messages, but not sure how that would impact the
gui.

https://code.launchpad.net/~thumper/juju-core/debug-log-api-client/+merge/214875

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/85850043/

Affected files (+322, -4 lines):
   A [revision details]
   M state/api/apiclient.go
   M state/api/client.go
   M state/api/client_test.go
   M state/api/export_test.go
   M state/apiserver/client/client.go
   M state/apiserver/client/client_test.go
   M state/apiserver/debuglog_test.go
   M utils/http.go
   M utils/http_test.go

« Back to merge proposal