feature: inspect network by ID #1040
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1040 +/- ##
==========================================
- Coverage 15.68% 15.61% -0.08%
==========================================
Files 139 139
Lines 8499 8538 +39
==========================================
Hits 1333 1333
- Misses 7066 7105 +39
Partials 100 100
Continue to review full report at Codecov.
|
daemon/mgr/network.go
Outdated
| return ok | ||
| } | ||
|
|
||
| // Get returns the information of network that specified name/id. |
There was a problem hiding this comment.
Could you add more function Get logic in the comments to help add readability?
daemon/mgr/network.go
Outdated
| } | ||
| matchedNetworks := nm.GetNetworksByID(id) | ||
| if len(matchedNetworks) == 0 { | ||
| return nil, errors.Wrap(errtypes.ErrNotfound, libnetwork.ErrNoSuchNetwork(id).Error()) |
There was a problem hiding this comment.
I do not think we should wrap this, since we have already indentify this is a libnetwork.ErrNoSuchNetwork(id).
Or remove the latter one to keep the errors.Wrap?
WDYT? @faycheng
daemon/mgr/network.go
Outdated
| } | ||
|
|
||
| if n != nil { | ||
| if err == nil { |
There was a problem hiding this comment.
If the logic is correct, I should suggest to use return fast to avoid the large ident. Like:
if err != nil {
return nil, err
}
return &types.Network{
......
}
There was a problem hiding this comment.
@allencloud Thanks a lot for your reviews.
I have modified the code according to your suggestions.
daemon/mgr/network.go
Outdated
| var matchedNetworks []*types.Network | ||
|
|
||
| walker := func(nw libnetwork.Network) bool { | ||
| if strings.HasPrefix(nw.ID(), id) { |
There was a problem hiding this comment.
I don't know why it appears the same prefix id of networks? Id is not uniquely identifies?
There was a problem hiding this comment.
@rudyfly
Thanks a lot for your reviews.
id represents prefix of network ID, or full network ID, and id is renamed partialID.
Sorry for making you confused.
Signed-off-by: 程飞 <fay.cheng.cn@gmail.com>
faycheng
force-pushed
the
inspect-network-by-id
branch
from
45eddc0 to
54dc0bb
Compare
|
Could you help to make this move on? @rudyfly |
|
LGTM |
pouchrobot
added
the
LGTM
Signed-off-by: 程飞 fay.cheng.cn@gmail.com
Ⅰ. Describe what this PR did
add network inspection with network ID
Ⅱ. Does this pull request fix one issue?
Ⅲ. Describe how you did it
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews