Fixing grouped node count for filtered children nodes #1371
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
paulbellamy
force-pushed
the
1219-grouped-node-counts-2
branch
3 times, most recently
from
a8525dd to
465aeb0
Compare
|
@tomwilkie, PTAL |
Perhaps we should include the containers state in the details panel child table? |
|
state, and whether or not it's a system container... |
paulbellamy
force-pushed
the
1219-grouped-node-counts-2
branch
from
57821d7 to
a426ca2
Compare
|
Seem to have lots the stats on the number of containers filtered (on the containers view) |
|
And needs a good squash. Otherwise, LGTM. |
paulbellamy
force-pushed
the
1219-grouped-node-counts-2
branch
6 times, most recently
from
682c02e to
3d2aff5
Compare
|
LGTM filter changes. |
Squash of: * We have to keep all the container hostnames until the end so we can count how many we've filtered * Adding tests for ContainerHostnameRenderer and PodServiceRenderer with filters * Because we filter on image name we need the image name before filtering * Alternative approach to passing decorators. * Refactor out some of the decorator capture * Don't memoise decorated calls to Render * Fixing filtered counts on containers topology Tricky, because we need the filters to be silent sometimes (when they're in the middle), but not when they're at the top, so we take the "top" filter's stats. However, this means we have to compose all user-specified filters into a single Filter layer, so we can get all stats. There are no more Silent filters, as all filters are silent (unless they are at the top). Additionally, I clarified some of the filters as their usage/terminology was inconsistent and confused. Now Filter(IsFoo, ...) *keeps* only nodes where IsFoo is true.
paulbellamy
force-pushed
the
1219-grouped-node-counts-2
branch
from
3d2aff5 to
3d3aed2
Compare
|
Squashed, will wait for circle, before merging |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1219, Fixes #1130
Does not fix #815
Filtered children are still shown in the details panel, because otherwise one of them stopping could make the detail panel go empty, when the node disappears, which is slightly odd. But this means that a container image could say "1 container" (if it has 4 stopped and one running), then show all 5 containers in the detail panel, which is also slightly odd.
I'm not really sure about the naming of
APITopologyDesc.filteredRenderer, or about having two different fields for this. I was hoping to have just one field, so all the renderers would have the same signature. But, since only some of them need the filters injected, it feels odd to force that requirement on the others.This will (at the moment) also break memoisation for some of the rendering pipelines. There is an assumption that the map/reduce pipelines survive between requests, but this is no longer the case for all pipelines.
Will squash stuff before merging.