Limit number of objects on topology views #95

zeari · 2017-01-08T14:43:44Z

Set object limit in settings:

Objects kinds are hidden by order of granularity. Since containers are the least major entity they will be hidden first, then pods, then similarly other types until we are below the set object limit.
Edit: you can bring back each hidden type separately (containers, pods, etc.) using the filter buttons on top

@dclarizio @simon3z The code here is bad but in general, this is a possible approach towards scaling in topology. If this policy is good enough Ill go forward with it.

zeari · 2017-01-08T14:44:25Z

@miq-bot add_label providers/containers, topology

miq-bot · 2017-01-08T14:45:16Z

@zeari Cannot apply the following label because they are not recognized: topology

zeari · 2017-01-08T14:48:23Z

Setting the default number of items per page for topology to 20 would yield:

simon3z · 2017-01-09T10:45:19Z

Approach LGTM 👍.

@dclarizio can you review the approach/screenshots? (Code can be improved when we'll move this out of WIP).

@miq-bot assign dclarizio

dclarizio · 2017-01-09T16:55:58Z

@simon3z I'm not sure I'm fully on board with the approach. I mean, in theory I like it, but I have some issues:

you can't tell if there are more nodes you are not seeing, so it could be misleading, especially if you see only some of a certain node type
if I'm at my limit and I try to turn on another node type, nothing will happen, so we may need to show an error message

When I was thinking about this earlier, I had an idea that we could limit the maximum number of child nodes of any type (per parent), then show an extra "group" node, something like 28 more....

Another limit setting, at least for the initial display, could be Child Node Depth, which would turn off any nodes more than X levels below the root.

In both cases, perhaps we could let the user then drill into (or turn on) missing nodes on demand if they really need to see them.

Adding @serenamarie125 for more input.

zeari · 2017-01-09T17:12:47Z

you can't tell if there are more nodes you are not seeing, so it could be misleading, especially if you see only some of a certain node type
if I'm at my limit and I try to turn on another node type, nothing will happen, so we may need to show an error message

@dclarizio This is not the case. In this solution, all of a certain type are hidden (meaning all containers\ all pods\ all of type) by default and you can bring them all back using the filtering buttons on top. Those filter buttons are just on by default. Its indicated by the greyed out icons and the user can return to have all types shown.

Node depth from provider(root node) might not be a better filtering method since there are many types that are 'the same depth' but are less granular or interesting than their counterparts.

dclarizio · 2017-01-09T17:44:11Z

@zeari ahhh, I see . .. I misunderstood . . . so we eliminate (turn off) types, based on the furthest out nodes (i.e. granularity) until below the configured limit, but the user can turn them back on if they want, cool.

I still think a max number of child nodes would be a good idea, such that if I know more than 50 nodes is impossible for me to see clearly, it would just show the 50 and then a 83 more node that I could drill into if I wanted to. Just an idea. 😄

zeari · 2017-01-10T16:26:23Z

I still think a max number of child nodes would be a good idea, such that if I know more than 50 nodes is impossible for me to see clearly, it would just show the 50 and then a 83 more node that I could drill into if I wanted to. Just an idea.

That might be nice. Im not sure how we would decide which entity to hide (node?container?pod?) or how many of them to hide.
Anyway, if this approach is good enough for now I can go forward with it @dclarizio

dclarizio · 2017-01-10T18:16:24Z

@zeari this will help and can potentially be improved on later, I'm on board with moving forward . . . Thx, Dan

serenamarie125 · 2017-01-13T22:33:14Z

@zeari Couple of questions ...

If they limit the number of objects to see by default, is there to way to bring some back into view incrementally without leaving the page?
Does it make sense to limit the number where you are ? It looks like it's the same place as setting number of items per page (as in paging)

zeari · 2017-01-15T09:30:06Z

If they limit the number of objects to see by default, is there to way to bring some back into view incrementally without leaving the page?

@serenamarie125 yes, you can bring back each hidden type separately (containers, pods, etc.) using the filter buttons on top

Does it make sense to limit the number where you are ? It looks like it's the same place as setting number of items per page (as in paging)

Maybe not. Where would you put it?

himdel · 2017-01-19T13:46:46Z

app/assets/javascripts/controllers/container_topology/container_topology_controller.js

@@ -37,6 +37,20 @@ function ContainerTopologyCtrl($scope, $http, $interval, $location, topologyServ

      if (currentSelectedKinds && (Object.keys(currentSelectedKinds).length != Object.keys($scope.kinds).length)) {
        $scope.kinds = currentSelectedKinds;
+      } else {
+        const remove_hierarchy = ['Container', 'ContainerGroup', 'ContainerReplicator', 'ContainerService',


can't use const yet, sorry :)

himdel · 2017-01-19T13:47:12Z

app/assets/javascripts/controllers/container_topology/container_topology_controller.js

+        const remove_hierarchy = ['Container', 'ContainerGroup', 'ContainerReplicator', 'ContainerService',
+                                  'ContainerRoute', 'Host', 'Vm', 'ContainerNode', 'ContainerManager'];
+        // Cant use Object.values...
+        var tmp_list = Object.keys($scope.items).map(function(key) { return $scope.items[key] });


Could be _.values ;)

himdel · 2017-01-19T13:47:42Z

app/assets/javascripts/controllers/container_topology/container_topology_controller.js

+        var kind_index = 0;
+        while ((tmp_list.length > data.data.size_limit) && kind_index < remove_hierarchy.length) {
+          kind_to_hide = remove_hierarchy[kind_index];
+          tmp_list = tmp_list.filter(function(item) {


Not a good idea to change tmp_list while iterating over it..

EDIT: ..unless you just care about the lenght :). We can keep this I think..

himdel · 2017-01-19T13:47:46Z

app/assets/javascripts/controllers/container_topology/container_topology_controller.js

+        var tmp_list = Object.keys($scope.items).map(function(key) { return $scope.items[key] });
+        var kind_index = 0;
+        while ((tmp_list.length > data.data.size_limit) && kind_index < remove_hierarchy.length) {
+          kind_to_hide = remove_hierarchy[kind_index];


missing var

himdel · 2017-01-19T13:48:16Z

app/views/configuration/_ui_1.html.haml

+             [_("Tile View"),    "perpage_tile",     :tile],
+             [_("List View"),    "perpage_list",     :list],
+             [_("Reports"),      "perpage_reports",  :reports],
+             [_("Topolgy View"), "perpage_topology", :topology]].each do |item_per_page|


Topolgy != Topology ;)

zeari · 2017-01-19T14:31:39Z

@miq-bot remove_label wip

@simon3z Aside from the comment by @serenamarie125 this should be ready.

serenamarie125 · 2017-01-19T15:58:09Z

@zeari i would suggest something more like this, where it's in a box on it's own.
Here i have a switch which allows them to say if they want the auto filtering first ... is this something you could do as well ? If set to off, then the drop down would be disabled

himdel · 2017-01-19T16:49:34Z

app/assets/javascripts/controllers/container_topology/container_topology_controller.js

@@ -37,6 +37,20 @@ function ContainerTopologyCtrl($scope, $http, $interval, $location, topologyServ

      if (currentSelectedKinds && (Object.keys(currentSelectedKinds).length != Object.keys($scope.kinds).length)) {
        $scope.kinds = currentSelectedKinds;
+      } else {
+        remove_hierarchy = ['Container', 'ContainerGroup', 'ContainerReplicator', 'ContainerService',


I guess this should be

var remove_hierarchy = [ 'Container', 'ContainerGroup', 'ContainerManager', 'ContainerNode', 'ContainerReplicator', 'ContainerRoute', 'ContainerService', 'Host', 'Vm', ];

(because missing var, improbable formatting, and easier to see if its one entry per line, sorted alphabetically)

himdel · 2017-01-25T09:46:48Z

spec/javascripts/fixtures/json/container_topology_response.json

-      "OpenshiftEnterprise":{"type":"image","icon":"...openshift_enterprise..."}}}
+      "OpenshiftEnterprise":{"type":"image","icon":"...openshift_enterprise..."}
+    },
+    "size_limit": 100


I think this is no longer the right data, hence the spec failure ;)

himdel · 2017-01-25T09:48:09Z

Thanks @zeari that looks really nice, should make it much easier to add to another topologies :).

I'd merge, except for the spec failure :)

simon3z · 2017-01-25T09:49:43Z

I'd merge, except for the spec failure :)

@himdel please note that above there's still a comment on the default value.

himdel · 2017-01-25T10:03:35Z

Ah, yeah, sorry about that, the default value is up to you guys, if that's something for a longer discussion, changing it would be trivial so .. I don't really consider it a blocker :) (But the current default would seem to be Unlimited.)

himdel · 2017-01-25T10:04:37Z

@zeari One more thing I notice .. if you enter the Settings screen with a clean (new) user, you'll get an exception...

undefined method '[]' for nil:NilClass [configuration/index]

Pretty sure it's that @edit[:new][:perpage][item_per_page[2]] in the ui_1 view.

(Note I also have to change entity.default_authentications.status.capitalize to ....try(:capitalize) (ContainerTopologyService#entity_status), but that's not a problem for this PR.)

miq-bot · 2017-01-25T10:28:03Z

This pull request is not mergeable. Please rebase and repush.

miq-bot · 2017-01-26T11:33:16Z

Checked commit zeari@b0862e5 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
4 files checked, 1 offense detected

app/controllers/configuration_controller.rb

❕ - Line 579, Col 121 - Metrics/LineLength - Line is too long. [145/120]

zeari · 2017-01-26T12:14:48Z

@zeari One more thing I notice .. if you enter the Settings screen with a clean (new) user, you'll get an exception...

undefined method '[]' for nil:NilClass [configuration/index]

Pretty sure it's that @edit[:new][:perpage][item_per_page[2]] in the ui_1 view.

@himdel I made a new user but im not getting this exception. ui_constants is reloaded when you reset the server so it might be that. topology setting was under perpage but isnt anymore so a restart might kick it off.

(Note I also have to change entity.default_authentications.status.capitalize to ....try(:capitalize) (ContainerTopologyService#entity_status), but that's not a problem for this PR.)

How did you add your containers provider? this shouldnt happen anymore since the ui doesnt let you add a containers provider that doesnt have a bearer auth.

zeari · 2017-01-26T13:06:37Z

@himdel please note that above there's still a comment on the default value.

@simon3z Changed default value to unlimited.

himdel · 2017-01-26T13:25:55Z

@himdel I made a new user but im not getting this exception. ui_constants is reloaded when you reset the server so it might be that. topology setting was under perpage but isnt anymore so a restart might kick it off.

You were right, sorry about that, it was indeed that restart thing :).

How did you add your containers provider? this shouldnt happen anymore since the ui doesnt let you add a containers provider that doesnt have a bearer auth.

Well, I'm not seeing any migrations that would fix providers that were added before it couldn't happen ;). But yeah, my db is not new.

himdel · 2017-01-26T13:27:32Z

Ok, this is working and looks good, merging.

abonas · 2017-05-14T13:40:30Z

@miq-bot add_label topology

miq-bot · 2017-05-14T13:41:21Z

@zeari Cannot apply the following label because they are not recognized: providers/containers

miq-bot added the compute/containers label Jan 8, 2017

zeari changed the title ~~[WIP] Limit number of objects on topology screens~~ [WIP] Limit number of objects on topology views Jan 8, 2017

miq-bot assigned dclarizio Jan 9, 2017

mzazrivec added the wip label Jan 9, 2017

zeari force-pushed the set_topology_object_limit branch from 1d5afba to 0b0b9e9 Compare January 12, 2017 12:57

zeari force-pushed the set_topology_object_limit branch 2 times, most recently from e1a0f07 to c3aafb3 Compare January 17, 2017 11:18

himdel reviewed Jan 19, 2017

View reviewed changes

zeari force-pushed the set_topology_object_limit branch 2 times, most recently from 95c819d to 74cc5e0 Compare January 19, 2017 14:15

zeari changed the title ~~[WIP] Limit number of objects on topology views~~ Limit number of objects on topology views Jan 19, 2017

miq-bot removed the wip label Jan 19, 2017

himdel reviewed Jan 19, 2017

View reviewed changes

himdel reviewed Jan 25, 2017

View reviewed changes

miq-bot added the unmergeable label Jan 25, 2017

Limit number of objects on topology screens

b0862e5

zeari force-pushed the set_topology_object_limit branch from 9e93b0b to b0862e5 Compare January 26, 2017 11:31

miq-bot removed the unmergeable label Jan 26, 2017

himdel added enhancement euwe/no labels Jan 26, 2017

himdel assigned himdel and unassigned dclarizio Jan 26, 2017

himdel merged commit 41742ee into ManageIQ:master Jan 26, 2017

himdel added this to the Sprint 53 Ending Jan 30, 2017 milestone Jan 26, 2017

This was referenced Jan 26, 2017

Set a default limit on the number of objects in each type of topology view #252

Closed

Reduce default container topology object limit #251

Merged

simon3z mentioned this pull request Feb 23, 2017

Topology for Container Projects #120

Merged

miq-bot added the topology label May 14, 2017

miq-bot assigned dclarizio and unassigned himdel May 14, 2017

zeari mentioned this pull request May 29, 2017

fix topology max items feature #1453

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Limit number of objects on topology views #95

Limit number of objects on topology views #95

zeari commented Jan 8, 2017 •

edited

Loading

zeari commented Jan 8, 2017

miq-bot commented Jan 8, 2017

zeari commented Jan 8, 2017

simon3z commented Jan 9, 2017

dclarizio commented Jan 9, 2017

zeari commented Jan 9, 2017 •

edited

Loading

dclarizio commented Jan 9, 2017

zeari commented Jan 10, 2017 •

edited

Loading

dclarizio commented Jan 10, 2017

serenamarie125 commented Jan 13, 2017

zeari commented Jan 15, 2017

himdel Jan 19, 2017

himdel Jan 19, 2017

himdel Jan 19, 2017 •

edited

Loading

himdel Jan 19, 2017

himdel Jan 19, 2017

zeari commented Jan 19, 2017 •

edited

Loading

serenamarie125 commented Jan 19, 2017

himdel Jan 19, 2017

himdel Jan 19, 2017

himdel Jan 25, 2017

himdel commented Jan 25, 2017

simon3z commented Jan 25, 2017

himdel commented Jan 25, 2017 •

edited

Loading

himdel commented Jan 25, 2017 •

edited

Loading

miq-bot commented Jan 25, 2017

miq-bot commented Jan 26, 2017

zeari commented Jan 26, 2017 •

edited

Loading

zeari commented Jan 26, 2017

himdel commented Jan 26, 2017

himdel commented Jan 26, 2017

abonas commented May 14, 2017

miq-bot commented May 14, 2017

Limit number of objects on topology views #95

Limit number of objects on topology views #95

Conversation

zeari commented Jan 8, 2017 • edited Loading

zeari commented Jan 8, 2017

miq-bot commented Jan 8, 2017

zeari commented Jan 8, 2017

simon3z commented Jan 9, 2017

dclarizio commented Jan 9, 2017

zeari commented Jan 9, 2017 • edited Loading

dclarizio commented Jan 9, 2017

zeari commented Jan 10, 2017 • edited Loading

dclarizio commented Jan 10, 2017

serenamarie125 commented Jan 13, 2017

zeari commented Jan 15, 2017

himdel Jan 19, 2017

Choose a reason for hiding this comment

himdel Jan 19, 2017

Choose a reason for hiding this comment

himdel Jan 19, 2017 • edited Loading

Choose a reason for hiding this comment

himdel Jan 19, 2017

Choose a reason for hiding this comment

himdel Jan 19, 2017

Choose a reason for hiding this comment

zeari commented Jan 19, 2017 • edited Loading

serenamarie125 commented Jan 19, 2017

himdel Jan 19, 2017

Choose a reason for hiding this comment

himdel Jan 19, 2017

Choose a reason for hiding this comment

himdel Jan 25, 2017

Choose a reason for hiding this comment

himdel commented Jan 25, 2017

simon3z commented Jan 25, 2017

himdel commented Jan 25, 2017 • edited Loading

himdel commented Jan 25, 2017 • edited Loading

miq-bot commented Jan 25, 2017

miq-bot commented Jan 26, 2017

zeari commented Jan 26, 2017 • edited Loading

zeari commented Jan 26, 2017

himdel commented Jan 26, 2017

himdel commented Jan 26, 2017

abonas commented May 14, 2017

miq-bot commented May 14, 2017

zeari commented Jan 8, 2017 •

edited

Loading

zeari commented Jan 9, 2017 •

edited

Loading

zeari commented Jan 10, 2017 •

edited

Loading

himdel Jan 19, 2017 •

edited

Loading

zeari commented Jan 19, 2017 •

edited

Loading

himdel commented Jan 25, 2017 •

edited

Loading

himdel commented Jan 25, 2017 •

edited

Loading

zeari commented Jan 26, 2017 •

edited

Loading