Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1318)

Issue 38420046: cmd/juju: status using the API (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 4 months ago by dimitern
Modified:
10 years, 4 months ago
Reviewers:
mp+199814, rog
Visibility:
Public.

Description

cmd/juju: status using the API Add status cmd to api with state access fallback Current progress on having `juju status` use the api. Moves the existing code to a resurrected state/statecmd file. This is exposed both through the api, and as a fallback to accessing state directly. The status tests now pass, but moving some testing down a layer or two would probably be a good plan. Is there anything else required for a status first pass? dimitern: I pulled Martin's branch lp:~gz/juju-core/status_api, merged trunk and resolved conflicts, also fixed a couple of tests, but no other changes are made. All tests pass, and I also did a successful live test on EC2. Original CL is at: https://codereview.appspot.com/40350043/, and has already 2 LGTMs. https://code.launchpad.net/~dimitern/juju-core/gz-status_api/+merge/199814 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 2

Patch Set 2 : cmd/juju: status using the API #

Unified diffs Side-by-side diffs Delta from patch set Stats (+772 lines, -540 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M cmd/juju/status.go View 6 chunks +114 lines, -470 lines 0 comments Download
M cmd/juju/status_test.go View 1 7 chunks +36 lines, -36 lines 0 comments Download
M environs/jujutest/livetests.go View 1 chunk +1 line, -1 line 0 comments Download
M state/api/client.go View 2 chunks +45 lines, -7 lines 0 comments Download
M state/api/params/params.go View 1 chunk +5 lines, -0 lines 0 comments Download
M state/apiserver/client/api_test.go View 1 chunk +70 lines, -4 lines 0 comments Download
M state/apiserver/client/client.go View 1 chunk +0 lines, -20 lines 0 comments Download
M state/apiserver/client/client_test.go View 1 chunk +1 line, -1 line 0 comments Download
M state/apiserver/client/perm_test.go View 1 chunk +1 line, -1 line 0 comments Download
A state/apiserver/client/status.go View 1 chunk +21 lines, -0 lines 0 comments Download
A state/statecmd/status.go View 1 chunk +476 lines, -0 lines 0 comments Download

Messages

Total messages: 3
dimitern
Please take a look.
10 years, 4 months ago (2013-12-20 14:04:00 UTC) #1
rog
LGTM with one additional added comment. https://codereview.appspot.com/38420046/diff/1/cmd/juju/status_test.go File cmd/juju/status_test.go (right): https://codereview.appspot.com/38420046/diff/1/cmd/juju/status_test.go#newcode69 cmd/juju/status_test.go:69: st := s.Conn.Environ.(testing.GetStater).GetStateInAPIServer() ...
10 years, 4 months ago (2013-12-20 14:08:19 UTC) #2
dimitern
10 years, 4 months ago (2013-12-20 14:30:16 UTC) #3
Please take a look.

https://codereview.appspot.com/38420046/diff/1/cmd/juju/status_test.go
File cmd/juju/status_test.go (right):

https://codereview.appspot.com/38420046/diff/1/cmd/juju/status_test.go#newcode69
cmd/juju/status_test.go:69: st :=
s.Conn.Environ.(testing.GetStater).GetStateInAPIServer()
On 2013/12/20 14:08:20, rog wrote:
> // We make changes in the API server's state so that
> // our changes to presence are immediately noticed
> // in the status.

Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b