feature: add network ls command #642
Conversation
apis/swagger.yml
Outdated
| - `id=<network-id>` Matches all or part of a network ID. | ||
| - `label=<key>` or `label=<key>=<value>` of a network label. | ||
| - `name=<network-name>` Matches all or part of a network name. | ||
| - `scope=["swarm"|"global"|"local"]` Filters networks by scope (`swarm`, `global`, or `local`). |
There was a problem hiding this comment.
Please remove all these filter things, since we do not use them in a quite long time.
|
Ci fails: |
Codecov Report
@@ Coverage Diff @@
## master #642 +/- ##
==========================================
- Coverage 14.21% 14.02% -0.19%
==========================================
Files 65 65
Lines 3736 3786 +50
==========================================
Hits 531 531
- Misses 3159 3209 +50
Partials 46 46
Continue to review full report at Codecov.
|
apis/swagger.yml
Outdated
| schema: | ||
| type: "array" | ||
| items: | ||
| $ref: "#/definitions/NetworkListResp" |
There was a problem hiding this comment.
I do not think this correct. Since in code you returned respNetworks := types.NetworkListResp{Networks: []*types.NetworkInfo{}, Warnings: nil}. But here you returned []types.NetworkListResp. They are not the same.
You have already defined Networks in NetworkListResp as an array. So you do not need to make this api to return an array of NetworkListResp. This is not correct, to be honest. @ZouRui89
allencloud
changed the title
ZouRui89
changed the title
|
please merge two commits. |
ZouRui89
changed the title
cli/network.go
Outdated
|
|
||
| // networkListExample shows examples in network list command, and is used in auto-generated cli docs. | ||
| func networkListExample() string { | ||
| return `` |
apis/server/network_bridge.go
Outdated
| respNetworks.Networks = append(respNetworks.Networks, &types.NetworkInfo{ | ||
| Name: net.Name, | ||
| ID: net.ID, | ||
| }) |
There was a problem hiding this comment.
Network driver is necessary.
cli/network.go
Outdated
| n.cli = c | ||
|
|
||
| n.cmd = &cobra.Command{ | ||
| Use: "list [OPTIONS] [NAME]", |
There was a problem hiding this comment.
Please remove [OPTIONS] [NAME], since you specified no flag and no args is accpeted.
| path = "/networks" | ||
| resp, err = request.Get(path) | ||
| c.Assert(err, check.IsNil) | ||
| c.Assert(resp.StatusCode, check.Equals, 200) |
There was a problem hiding this comment.
Could also verify the Name, Driver field.
There was a problem hiding this comment.
This is test for list API, not inspect API. So maybe we add this in inspect API.
|
Updated @allencloud |
cli/network.go
Outdated
| display.AddRow([]string{"NETWORK ID", "NAME", "DRIVER", "SCOPE"}) | ||
| for _, network := range resp.Networks { | ||
| display.AddRow([]string{ | ||
| network.ID, |
There was a problem hiding this comment.
change the id to short id is better？
There was a problem hiding this comment.
I think short ID is better, since the original ones is too long. And it may make output in cli not so readable.
There was a problem hiding this comment.
I agree with you two.
Signed-off-by: Zou Rui <[email protected]>
|
LGTM |
pouchrobot
added
the
LGTM
feature: add network ls implementation on API side: (AliyunContainerService#642)
Signed-off-by: Zou Rui [email protected]
1.Describe what this PR did
add network ls implementation on API side
2.Does this pull request fix one issue?
NONE
3.Describe how you did it
4.Describe how to verify it
5.Special notes for reviews