List Networks need not pull all the endpoints#30673
Conversation
|
Looks like this is a change to the API though 😞 |
|
I'm going to pull this off merge, because the change indeed needs to be addressed in some way. |
|
@cpuguy83 @thaJeztah i think the original implementation is a bug and we are fixing it the correct way here. I have seen other bugs filed for the exact same issue (I will point them when i find it). Also the inspect APIs are available to get the data. I don't think this change is going to break most of the users. |
|
@mavenugo true, but this might break people and we should follow a depreciation policy as we do usually. We need, at least, to version this change (so using the same API version endpoint give the same result) |
|
@vdemeester @thaJeztah @cpuguy83 @tiborvass Added the version check. PTAL. |
There was a problem hiding this comment.
not tryin to bikeshed but i find the function name a bit weird. How about buildDetailedNetworkResource since NetworkResource is the normal one.
Pulling all the endpoints is a very resource heavy operation especially for Global-scoped networks with a backing KVStore. Such heavy operations can be fetched for individual network inspect. These are unneccessary for a simple network list operation. Signed-off-by: Madhu Venugopal <madhu@docker.com>
|
@tiborvass done changing the function name. ptal. |
| list = append(list, *nr) | ||
| } | ||
|
|
||
| list, err = filterNetworks(list, netFilters) |
There was a problem hiding this comment.
As a follow up, it may be worth considering to add information not needed for filtering until after the networks have been filtered.
Perhaps create utility functions to append information such as containers and peers (e.g. appendEndpointInfo(list), appendPeerInfo(list))
thaJeztah
left a comment
There was a problem hiding this comment.
left a suggestion for possible improvement (follow up), but LGTM on this one
@tiborvass LGTY?
|
LGTM |
Pulling all the endpoints is a very resource heavy operation especially
for Global-scoped networks with a backing KVStore. Such heavy operations
can be fetched for individual network inspect. These are unneccessary
for a simple network list operation.
Signed-off-by: Madhu Venugopal madhu@docker.com