Conversation
@@ -18,6 +19,22 @@ type APIClient interface { | |||
CheckpointCreate(ctx context.Context, container string, options types.CheckpointCreateOptions) error | |||
CheckpointDelete(ctx context.Context, container string, checkpointID string) error | |||
CheckpointList(ctx context.Context, container string) ([]types.Checkpoint, error) | |||
SwarmInit(ctx context.Context, req swarm.InitRequest) (string, error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The APIClient
interface is getting bigger and bigger. We might want to split it into several ones (like VolumeAPIClient
, NetworkAPIClient
, SwarmAPIClient
, …) and have APIClient
embedded them…
It's a nit and not in the scope of the PR… just thinking outloud.
This would help project using engine-api
to have less work to do to unit-test some part 👼
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we should definitely do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, I'll prepare a follow-up for this then 😉
thanks for the review @vdemeester I'll push some changes soon |
LGTM nice looking code +1 |
) | ||
|
||
// IPAMConfigFamily represents the family of a ipam configuration. | ||
type IPAMConfigFamily string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Family can be detected by the format of subnet. Can we remove this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes we can remove that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mrjana ok I'm removing it, can you remove it from swarmkit ?
Signed-off-by: Victor Vieux <vieux@docker.com>
Signed-off-by: Victor Vieux <vieux@docker.com>
- TaskInspect - TaskList Signed-off-by: Vincent Demeester <vincent@sbr.pm>
- SwarmInit - SwarmInspect - SwarmJoin - SwarmLeave - SwarmUpdate Signed-off-by: Vincent Demeester <vincent@sbr.pm>
- ServiceCreate - ServiceInspect - ServiceList - ServiceRemove - ServiceUpdate Signed-off-by: Vincent Demeester <vincent@sbr.pm>
- NodeInspect - NodeList - NodeRemove - NodeUpdate Signed-off-by: Vincent Demeester <vincent@sbr.pm>
remove Reserved add CACertHash cleanup of types Signed-off-by: Victor Vieux <vieux@docker.com> Signed-off-by: Stephen J Day <stephen.day@docker.com>
Signed-off-by: Victor Vieux <vieux@docker.com> Signed-off-by: Arnaud Porterie (icecrime) <arnaud.porterie@docker.com> Signed-off-by: Victor Vieux <vieux@docker.com>
Signed-off-by: Victor Vieux <vieux@docker.com>
LGTM |
ah golint erro
|
// NetworkAttachmentConfig represents the configuration of a network attachement. | ||
type NetworkAttachmentConfig struct { | ||
Target string `json:",omitempty"` | ||
Aliases []string `json:",omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 |
LGTM 99🎈 |
LGTM 💯 |
LGTM |
As described in our ROADMAP.md, introduce new swarmkit management API endpoints and types.
The new concepts introduced are: