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

[Benji York]

Add service constraint fetching to the API

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

[Dimiter Naydenov (dimitern)]

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/

[Brad Crittenden (bac)]

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/

[Brad Crittenden (bac)]

LGTM, FWIW.

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

=== 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 @@
 	if c.ServiceName == "" {
 		cons, err = conn.State.EnvironConstraints()
 	} else {
-		var svc *state.Service
-		if svc, err = conn.State.Service(c.ServiceName); err != nil {
-			return err
+		args := params.GetServiceConstraints{
+			ServiceName: c.ServiceName,
 		}
-		cons, err = svc.Constraints()
+
+		results, err = statecmd.GetServiceConstraints(conn.State, args)
+		cons = results.Constraints
 	}
 	if err != nil {
 		return err

=== 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 @@
 	return clientError(c.st.client.Call("Client", "", "ServiceDestroy", params, nil))
 }
 
+// GetServiceConstraints returns the constraints for the given service.
+func (c *Client) GetServiceConstraints(service string) (constraints.Value, error) {
+	results := new(params.GetServiceConstraintsResults)
+	err := c.st.client.Call("Client", "", "GetServiceConstraints", params.GetServiceConstraints{service}, results)
+	return results.Constraints, clientError(err)
+}
+
 // SetServiceConstraints specifies the constraints for the given service.
 func (c *Client) SetServiceConstraints(service string, constraints constraints.Value) error {
 	params := params.SetServiceConstraints{

=== 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 @@
 	Pairs map[string]string
 }
 
+// GetServiceConstraints stores parameters for making the GetServiceConstraints call.
+type GetServiceConstraints struct {
+	ServiceName string
+}
+
+// GetServiceConstraintsResults holds results of the GetServiceConstraints call.
+type GetServiceConstraintsResults struct {
+	Constraints constraints.Value
+}
+
 // SetServiceConstraints stores parameters for making the SetServiceConstraints call.
 type SetServiceConstraints struct {
 	ServiceName string

=== 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 @@
 	op:    opClientServiceDestroy,
 	allow: []string{"user-admin", "user-other"},
 }, {
+	about: "Client.GetServiceConstraints",
+	op:    opClientGetServiceConstraints,
+	allow: []string{"user-admin", "user-other"},
+}, {
 	about: "Client.SetServiceConstraints",
 	op:    opClientSetServiceConstraints,
 	allow: []string{"user-admin", "user-other"},
@@ -446,6 +450,13 @@
 	return func() {}, err
 }
 
+func opClientGetServiceConstraints(c *C, st *api.State, mst *state.State) (func(), error) {
+	// This test only checks that the call is made without error, ensuring the
+	// signatures match.
+	_, err := st.Client().GetServiceConstraints("wordpress")
+	return func() {}, err
+}
+
 func opClientSetServiceConstraints(c *C, st *api.State, mst *state.State) (func(), error) {
 	// This test only checks that the call is made without error, ensuring the
 	// signatures match.

=== 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 @@
 	return statecmd.ServiceDestroy(c.root.srv.state, args)
 }
 
+// GetServiceConstraints returns the constraints for a given service.
+func (c *srvClient) GetServiceConstraints(args params.GetServiceConstraints) (params.GetServiceConstraintsResults, error) {
+	return statecmd.GetServiceConstraints(c.root.srv.state, args)
+}
+
 // SetServiceConstraints sets the constraints for a given service.
 func (c *srvClient) SetServiceConstraints(args params.SetServiceConstraints) error {
 	return statecmd.SetServiceConstraints(c.root.srv.state, args)

=== 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 @@
+// Code shared by the CLI and API for the GetConstraints function.
+
+package statecmd
+
+import (
+	"launchpad.net/juju-core/constraints"
+	"launchpad.net/juju-core/state"
+	"launchpad.net/juju-core/state/api/params"
+)
+
+// GetServiceConstraints returns the constraints for a given service
+func GetServiceConstraints(st *state.State, args params.GetServiceConstraints) (params.GetServiceConstraintsResults, error) {
+	svc, err := st.Service(args.ServiceName)
+	if err != nil {
+		return params.GetServiceConstraintsResults{constraints.Value{}}, err
+	}
+	constraints, err := svc.Constraints()
+	return params.GetServiceConstraintsResults{constraints}, err
+}