service: --params flag retrieves service instance params #2141
Conversation
|
We have created an issue in Pivotal Tracker to manage this: https://www.pivotaltracker.com/story/show/176964913 The labels on this github issue will be updated when the story is started. |
| @@ -97,6 +93,26 @@ func (actor Actor) GetServiceInstanceDetails(serviceInstanceName string, spaceGU | |||
| return serviceInstanceDetails, Warnings(warnings), nil | |||
| } | |||
|
|
|||
| func (actor Actor) GetServiceInstanceParameters(serviceInstanceName string, spaceGUID string) (ServiceInstanceDetails, Warnings, error) { | |||
There was a problem hiding this comment.
It feels like a GetServiceInstanceParameters() function should return ServiceInstanceParameters rather than ServiceInstanceDetails
| Expect(fakeCloudControllerClient.GetServiceInstanceParametersArgsForCall(0)).To(Equal(serviceInstanceGUID)) | ||
| }) | ||
|
|
||
| When("getting the parameters fails with a V3UnexpectedResponseError", func() { |
There was a problem hiding this comment.
I don't think we should have a specific code path for a V3UnexpectedResponseError
| }` | ||
| }) | ||
|
|
||
| It("returns a BadRequestError", func() { |
There was a problem hiding this comment.
Should this say ServiceInstanceParametersFetchNotSupportedError?
command/v7/service_command.go
Outdated
| cmd.UI.DisplayNewline() | ||
|
|
||
| data, err := json.Marshal(cmd.serviceInstance.Parameters.Value) | ||
| data, err := json.MarshalIndent(cmd.serviceInstance.Parameters.Value, "", " ") | ||
| if err != nil { | ||
| panic(err) |
There was a problem hiding this comment.
I think I wrote this originally. What do you think about it? The reasoning is that since the parameters were once JSON, it's impossible for the Marshal to return an error - since errors would be because of types that cannot be marshalled. However looking at it again, it looks like a code small. I wonder if there's a better way to encapsulate it? Or just return the error so that the code looks "normal" even though there will never be an error?
There was a problem hiding this comment.
Hmm - What about just ignoring the error. We are confident that it is JSON and there are tests around the result.
command/v7/service_command_test.go
Outdated
| @@ -738,6 +653,101 @@ var _ = Describe("service command", func() { | |||
| }) | |||
| }) | |||
|
|
|||
| When("the --params flag is specified", func() { | |||
| const ( | |||
| parameters = `{"foo":"bar"}` | |||
| Eventually(helpers.CF(serviceCommand, serviceInstanceName)).Should(Say(`status:\s+create succeeded`)) | ||
| }) | ||
|
|
||
| FIt("reports the service instance parameters JSON", func() { |
| @@ -33,8 +33,7 @@ type SharedStatus struct { | |||
| } | |||
|
|
|||
| type ServiceInstanceParameters struct { | |||
There was a problem hiding this comment.
Would this be better?:
type ServiceInstanceParameters types.JSONObjectThere was a problem hiding this comment.
Or just use the types.JSONObject type...
| } | ||
|
|
||
| return ServiceInstanceParameters{Value: params}, warnings | ||
| return ServiceInstanceParameters{Value: params}, warnings, nil |
There was a problem hiding this comment.
Given that this is just one line, I'd move it into the switch statement, and then there's no need for a return at the end of the function.
5d2548b to
57b5c03
Compare
57b5c03 to
d954c1b
Compare
#173917704