Create api types package for structured responses #10986
Conversation
Move the stats structs from the api/stats package into a new package api/types that will contain all the api structs for docker's api request and responses. Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
| var result engine.Env | ||
| if err := result.Decode(stream); err != nil { | ||
| var response *types.ContainerCreateResponse | ||
| if err := json.NewDecoder(stream).Decode(&response); err != nil { |
There was a problem hiding this comment.
You use double pointer tactic here, but not checking for nilness. Looks like potential panic.
crosbymichael
force-pushed
the
create-api-type
branch
from
766d369
to
9472bce
Compare
|
ping @jfrazelle @tiborvass @LK4D4 |
| // creation of a new container. | ||
| type ContainerCreateResponse struct { | ||
| // ID is the id of the created container. | ||
| ID string `json:"Id"` |
There was a problem hiding this comment.
Why we need such tag? I vote for using default tags, it is simpler.
There was a problem hiding this comment.
+1, it's case insensitive by default: http://play.golang.org/p/jhQA8kZpXr
There was a problem hiding this comment.
Explicit is better than implicit especially when we want to enforce API consistency and make sure that the tags don't change the responses. With the tag i know that for a 100% fact I will get the marshaled response that I want.
There was a problem hiding this comment.
Hmm, you should be sure by test, not by some string, which also easy to misspell. We had such "100% fact" already in devicemapper code with json: "Id".
But I'm okay if you want save them.
This type is produced on the server side and is a type safe struct that can be encoded to json. It is consumed via the client. Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
crosbymichael
force-pushed
the
create-api-type
branch
from
9472bce
to
f57c265
Compare
|
COOL! |
|
LGTM |
1 similar comment
|
LGTM |
Create api types package for structured responses
This creates a new package
api/typesand also implements a new type for the/containers/createendpoint which is also consumed via the client.