Skip to content
This repository has been archived by the owner on Nov 23, 2019. It is now read-only.

Remove SwarmInspect method. #318

Closed
wants to merge 1 commit into from
Closed

Remove SwarmInspect method. #318

wants to merge 1 commit into from

Conversation

vdemeester
Copy link
Contributor

SwarmInspect information are redundant with Info. The API won't serve
GET /swarm anymore so removing the client method too. 👼

Related PR moby/moby#24492

/cc @thaJeztah @icecrime @tiborvass @aluzzardi @stevvooe

🐸

Signed-off-by: Vincent Demeester vincent@sbr.pm

SwarmInspect information are redundant with Info. The API won't serve
`GET /swarm` anymore so removing the client method too.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
@thaJeztah
Copy link
Member

LGTM

@icecrime
Copy link

LGTM

@@ -120,6 +120,8 @@ type Info struct {
Nodes int
Managers int
CACertHash string

Cluster Swarm
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Cluster contains data about the swarm to which this node has joined.

@stevvooe
Copy link
Contributor

Are we making a mistake by creating a compound data structures? Info is really about the node's local configuration, whereas cluster is about the configuration of the cluster.

The main issue here is that we end up with an imbalance between the object that is modified and how to read the results of that modification.

@vdemeester
Copy link
Contributor Author

The main issue here is that we end up with an imbalance between the object that is modified and how to read the results of that modification.

Yeah, I do agree on that one, it feels a little weird to update swarm properties via /swarm/* and getting swarm properties through /info..

Maybe we should keep swarm inspect and endpoint and just make docker info call the endpoint.

@tiborvass
Copy link
Contributor

tiborvass commented Jul 15, 2016

This tableflip is not specific to this issue, I'm just tired with all the back and forths that's all. Feel free to ignore it.

@stevvooe
Copy link
Contributor

@tiborvass This is not back and forth. This is CLI decisions being used to make thoughtless changes to the API. We have a deep design problem that our API and CLI are over-coupled. I'm fine with the CLI decision. It is a good one. I just don't understand why we think that means we have to delete this endpoint on the API.

Go ahead and merge this.

@vdemeester vdemeester closed this Jul 16, 2016
@vdemeester vdemeester deleted the remove-swarm-inspect branch July 16, 2016 07:22
@aluzzardi
Copy link
Contributor

Hey, side note - I think we need to be able to inspect in order to update (which we use for swarm config, such as join secret etc)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants