Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add filtering options to views; add options for filtering out system containers & unconnected containers. #420

Merged
merged 2 commits into from
Sep 7, 2015

Conversation

peterbourgon
Copy link
Contributor

Me and @davkal

@peterbourgon peterbourgon mentioned this pull request Sep 1, 2015
@@ -354,3 +356,62 @@ func (f Filter) Render(rpt report.Report) RenderableNodes {
}
return output
}

// FilterSystem is a Renderer which filters out system nodes.
func FilterSystem(r Renderer) Renderer {

This comment was marked as abuse.

This comment was marked as abuse.

@tomwilkie
Copy link
Contributor

Not using the DSL for this?

@peterbourgon
Copy link
Contributor Author

Not using the DSL for this?

It turned out mapping between report.Topology (what the DSL expects) and RenderableNodes (what the rendering pipeline outputs, and what the UI needs) was quite a bit of work, some of which was subtle. In the interest of pushing the feature, I went with this approach instead.

It will be simpler to have a full-expression renderer, i.e. only dumping to RenderableNodes at the end. That's what I'm targeting for the Kubernetes integration.

@peterbourgon peterbourgon force-pushed the without-system-containers branch from 7b927f8 to a39ec0a Compare September 2, 2015 07:48
@peterbourgon peterbourgon changed the title [WIP] Without system containers Without system containers Sep 3, 2015
@tomwilkie
Copy link
Contributor

@peterbourgon what happened to using labels instead of hardcoding the container names?

@peterbourgon
Copy link
Contributor Author

@tomwilkie we are using labels (see here) just not exclusively, as current weave master still doesn't put system labels on weave components.

@tomwilkie
Copy link
Contributor

as current weave master still doesn't put system labels on weave components.

It should do (I made that change!) - what version are you running?

@tomwilkie tomwilkie force-pushed the without-system-containers branch from 814b141 to 2d722ea Compare September 4, 2015 15:06
@tomwilkie tomwilkie changed the title Without system containers Add filtering options to views; add options for filtering out system containers & unconnected containers. Sep 4, 2015
@@ -156,6 +172,14 @@ const AppStore = assign({}, EventEmitter.prototype, {
AppStore.registeredCallback = function(payload) {
switch (payload.type) {

case ActionTypes.CHANGE_TOPOLOGY_OPTION:
if (activeTopologyOptions[payload.option] !== payload.value) {
nodes = {};

This comment was marked as abuse.

This comment was marked as abuse.

@davkal
Copy link
Contributor

davkal commented Sep 4, 2015

Not getting any errors anymore.

@tomwilkie
Copy link
Contributor

Thanks; I'm just adding some tests but this is feeling really close now.

@tomwilkie tomwilkie force-pushed the without-system-containers branch from ba37eb7 to 132cffa Compare September 4, 2015 16:18
@tomwilkie
Copy link
Contributor

Okay I'm done; its looking good.

One issue: I don't think the UI is selecting the default filter - I've changed the default to filter system containers, but I'm still seeing them in the UI. David, can you fix?

@tomwilkie tomwilkie assigned tomwilkie and davkal and unassigned tomwilkie Sep 4, 2015
@tomwilkie tomwilkie mentioned this pull request Sep 4, 2015
peterbourgon and others added 2 commits September 7, 2015 14:24
…state

- fix nodes reset after topology option change
- set topology on click
- explicitly set default topology options
- set default topo options when topos are received
- prevent JS error on too many nodes layout abort
@tomwilkie tomwilkie force-pushed the without-system-containers branch from 20d5abe to 27caf0a Compare September 7, 2015 14:31
@peterbourgon
Copy link
Contributor Author

LGTM!

peterbourgon added a commit that referenced this pull request Sep 7, 2015
Add filtering options to views; add options for filtering out system containers & unconnected containers.
@peterbourgon peterbourgon merged commit 3a48abf into master Sep 7, 2015
@peterbourgon peterbourgon deleted the without-system-containers branch September 7, 2015 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants