juju-core

Merge lp:~benji/juju-core/expose-service-constraints into lp:~juju/juju-core/trunk

  1. expose-service-constraints
  2. Merge into trunk
Proposed by Benji York
Status: Merged
Merged at revision: 1109
Proposed branch: lp:~benji/juju-core/expose-service-constraints
Merge into: lp:~juju/juju-core/trunk
Diff against target: 128 lines (+57/-4)
6 files modified
cmd/juju/constraints.go (+5/-4)
state/api/apiclient.go (+7/-0)
state/api/params/params.go (+10/-0)
state/apiserver/api_test.go (+11/-0)
state/apiserver/apiserver.go (+5/-0)
state/statecmd/getconstraints.go (+19/-0)
To merge this branch: bzr merge lp:~benji/juju-core/expose-service-constraints
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+157115@code.launchpad.net

Description of the change

Add service constraint fetching to the API

https://codereview.appspot.com/8366043/

To post a comment you must log in.
Revision history for this message
Dimiter Naydenov (dimitern) wrote :

LGTM, modulo a few trivials.

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

https://codereview.appspot.com/8366043/diff/1/cmd/juju/constraints.go#newcode66
cmd/juju/constraints.go:66: if err != nil {
how about just: if err == nil { cons = results.Constraints }, the if
below will take care of the other case.

https://codereview.appspot.com/8366043/diff/1/state/api/apiclient.go
File state/api/apiclient.go (right):

https://codereview.appspot.com/8366043/diff/1/state/api/apiclient.go#newcode171
state/api/apiclient.go:171: // GetServiceConstraints specifies the
constraints for the given service.
s/specifies/returns/

https://codereview.appspot.com/8366043/diff/1/state/apiserver/apiserver.go
File state/apiserver/apiserver.go (right):

https://codereview.appspot.com/8366043/diff/1/state/apiserver/apiserver.go#newcode374
state/apiserver/apiserver.go:374: // GetServiceConstraints sets the
constraints for a given service.
s/sets/returns/

https://codereview.appspot.com/8366043/diff/1/state/statecmd/getconstraints.go
File state/statecmd/getconstraints.go (right):

https://codereview.appspot.com/8366043/diff/1/state/statecmd/getconstraints.go#newcode11
state/statecmd/getconstraints.go:11: // GetServiceContstraints sets the
constraints for a given service
ditto

https://codereview.appspot.com/8366043/

Revision history for this message
Brad Crittenden (bac) wrote :

The code looks good, except for a few typos, etc.

There is quite a paucity of tests in this branch. Is your reasoning
that the call is just a wrapper around an already well-tested function?

https://codereview.appspot.com/8366043/diff/11001/cmd/juju/constraints.go
File cmd/juju/constraints.go (right):

https://codereview.appspot.com/8366043/diff/11001/cmd/juju/constraints.go#newcode66
cmd/juju/constraints.go:66: var results
params.GetServiceConstraintsResults
Is the explicit declaration required?

https://codereview.appspot.com/8366043/diff/11001/state/statecmd/getconstraints.go
File state/statecmd/getconstraints.go (right):

https://codereview.appspot.com/8366043/diff/11001/state/statecmd/getconstraints.go#newcode1
state/statecmd/getconstraints.go:1: // Code shared by the CLI and API
for the SetConstraints function.
GetConstraints

https://codereview.appspot.com/8366043/diff/11001/state/statecmd/getconstraints.go#newcode11
state/statecmd/getconstraints.go:11: // GetServiceContstraints returns
the constraints for a given service
typo: GetServiceContstraints

https://codereview.appspot.com/8366043/

Revision history for this message
Brad Crittenden (bac) wrote :

LGTM, FWIW.

https://codereview.appspot.com/8366043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'cmd/juju/constraints.go'
--- cmd/juju/constraints.go 2013-03-20 16:44:16 +0000
+++ cmd/juju/constraints.go 2013-04-04 15:56:21 +0000
@@ -59,11 +59,12 @@
59 if c.ServiceName == "" {59 if c.ServiceName == "" {
60 cons, err = conn.State.EnvironConstraints()60 cons, err = conn.State.EnvironConstraints()
61 } else {61 } else {
62 var svc *state.Service62 args := params.GetServiceConstraints{
63 if svc, err = conn.State.Service(c.ServiceName); err != nil {63 ServiceName: c.ServiceName,
64 return err
65 }64 }
66 cons, err = svc.Constraints()65
66 results, err = statecmd.GetServiceConstraints(conn.State, args)
67 cons = results.Constraints
67 }68 }
68 if err != nil {69 if err != nil {
69 return err70 return err
7071
=== modified file 'state/api/apiclient.go'
--- state/api/apiclient.go 2013-04-01 15:47:49 +0000
+++ state/api/apiclient.go 2013-04-04 15:56:21 +0000
@@ -168,6 +168,13 @@
168 return clientError(c.st.client.Call("Client", "", "ServiceDestroy", params, nil))168 return clientError(c.st.client.Call("Client", "", "ServiceDestroy", params, nil))
169}169}
170170
171// GetServiceConstraints returns the constraints for the given service.
172func (c *Client) GetServiceConstraints(service string) (constraints.Value, error) {
173 results := new(params.GetServiceConstraintsResults)
174 err := c.st.client.Call("Client", "", "GetServiceConstraints", params.GetServiceConstraints{service}, results)
175 return results.Constraints, clientError(err)
176}
177
171// SetServiceConstraints specifies the constraints for the given service.178// SetServiceConstraints specifies the constraints for the given service.
172func (c *Client) SetServiceConstraints(service string, constraints constraints.Value) error {179func (c *Client) SetServiceConstraints(service string, constraints constraints.Value) error {
173 params := params.SetServiceConstraints{180 params := params.SetServiceConstraints{
174181
=== modified file 'state/api/params/params.go'
--- state/api/params/params.go 2013-04-03 09:39:05 +0000
+++ state/api/params/params.go 2013-04-04 15:56:21 +0000
@@ -162,6 +162,16 @@
162 Pairs map[string]string162 Pairs map[string]string
163}163}
164164
165// GetServiceConstraints stores parameters for making the GetServiceConstraints call.
166type GetServiceConstraints struct {
167 ServiceName string
168}
169
170// GetServiceConstraintsResults holds results of the GetServiceConstraints call.
171type GetServiceConstraintsResults struct {
172 Constraints constraints.Value
173}
174
165// SetServiceConstraints stores parameters for making the SetServiceConstraints call.175// SetServiceConstraints stores parameters for making the SetServiceConstraints call.
166type SetServiceConstraints struct {176type SetServiceConstraints struct {
167 ServiceName string177 ServiceName string
168178
=== modified file 'state/apiserver/api_test.go'
--- state/apiserver/api_test.go 2013-04-01 19:54:06 +0000
+++ state/apiserver/api_test.go 2013-04-04 15:56:21 +0000
@@ -136,6 +136,10 @@
136 op: opClientServiceDestroy,136 op: opClientServiceDestroy,
137 allow: []string{"user-admin", "user-other"},137 allow: []string{"user-admin", "user-other"},
138}, {138}, {
139 about: "Client.GetServiceConstraints",
140 op: opClientGetServiceConstraints,
141 allow: []string{"user-admin", "user-other"},
142}, {
139 about: "Client.SetServiceConstraints",143 about: "Client.SetServiceConstraints",
140 op: opClientSetServiceConstraints,144 op: opClientSetServiceConstraints,
141 allow: []string{"user-admin", "user-other"},145 allow: []string{"user-admin", "user-other"},
@@ -446,6 +450,13 @@
446 return func() {}, err450 return func() {}, err
447}451}
448452
453func opClientGetServiceConstraints(c *C, st *api.State, mst *state.State) (func(), error) {
454 // This test only checks that the call is made without error, ensuring the
455 // signatures match.
456 _, err := st.Client().GetServiceConstraints("wordpress")
457 return func() {}, err
458}
459
449func opClientSetServiceConstraints(c *C, st *api.State, mst *state.State) (func(), error) {460func opClientSetServiceConstraints(c *C, st *api.State, mst *state.State) (func(), error) {
450 // This test only checks that the call is made without error, ensuring the461 // This test only checks that the call is made without error, ensuring the
451 // signatures match.462 // signatures match.
452463
=== modified file 'state/apiserver/apiserver.go'
--- state/apiserver/apiserver.go 2013-04-02 20:24:00 +0000
+++ state/apiserver/apiserver.go 2013-04-04 15:56:21 +0000
@@ -371,6 +371,11 @@
371 return statecmd.ServiceDestroy(c.root.srv.state, args)371 return statecmd.ServiceDestroy(c.root.srv.state, args)
372}372}
373373
374// GetServiceConstraints returns the constraints for a given service.
375func (c *srvClient) GetServiceConstraints(args params.GetServiceConstraints) (params.GetServiceConstraintsResults, error) {
376 return statecmd.GetServiceConstraints(c.root.srv.state, args)
377}
378
374// SetServiceConstraints sets the constraints for a given service.379// SetServiceConstraints sets the constraints for a given service.
375func (c *srvClient) SetServiceConstraints(args params.SetServiceConstraints) error {380func (c *srvClient) SetServiceConstraints(args params.SetServiceConstraints) error {
376 return statecmd.SetServiceConstraints(c.root.srv.state, args)381 return statecmd.SetServiceConstraints(c.root.srv.state, args)
377382
=== added file 'state/statecmd/getconstraints.go'
--- state/statecmd/getconstraints.go 1970-01-01 00:00:00 +0000
+++ state/statecmd/getconstraints.go 2013-04-04 15:56:21 +0000
@@ -0,0 +1,19 @@
1// Code shared by the CLI and API for the GetConstraints function.
2
3package statecmd
4
5import (
6 "launchpad.net/juju-core/constraints"
7 "launchpad.net/juju-core/state"
8 "launchpad.net/juju-core/state/api/params"
9)
10
11// GetServiceConstraints returns the constraints for a given service
12func GetServiceConstraints(st *state.State, args params.GetServiceConstraints) (params.GetServiceConstraintsResults, error) {
13 svc, err := st.Service(args.ServiceName)
14 if err != nil {
15 return params.GetServiceConstraintsResults{constraints.Value{}}, err
16 }
17 constraints, err := svc.Constraints()
18 return params.GetServiceConstraintsResults{constraints}, err
19}

Subscribers

People subscribed via source and target branches