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 Graylog cluster overview #2291

Merged
merged 6 commits into from May 26, 2016

Conversation

Projects
None yet
2 participants
@edmundoa
Member

edmundoa commented May 26, 2016

This PR adds a Graylog cluster overview component to the System -> Overview page. It also takes care of unifying some small things among all components being used in that page.

The changes should also be merged into master.

edmundoa added some commits May 26, 2016

Unify titles in system overview page
- Use same cases
- Remove icons from some of the titles. This was the only place where
  they didn't go away
Unify the way components load in system overview
Before, some components rendered a loading spinner without drawing a
container, but some didn't.

Now each component shows a container for its data, including the title and
static information. Spinners or content are included into that
container.
@@ -43,6 +43,14 @@ const NodesStore = Reflux.createStore({
return this.nodes[nodeId];
},
getClusterId() {
return Object.keys(this.nodes).map(id => this.nodes[id]).map(node => node.cluster_id)[0].toUpperCase();

This comment has been minimized.

@dennisoelkers

dennisoelkers May 26, 2016

Member

Why not doing something like .find(clusterId => clusterId) instead of [0], so we protect against possible undefined/null cluster ids?

This comment has been minimized.

@edmundoa
},
_onNodesChange() {
this.setState({ clusterId: NodesStore.getClusterId(), nodeCount: NodesStore.getNodeCount() });

This comment has been minimized.

@dennisoelkers

dennisoelkers May 26, 2016

Member

Why are the cluster id and the node count not included in the triggered state of the store, so we can just connect() to the store? We would also save those getters in the NodesStore which are kinda against Reflux principles (at least if they're avoidable).

This comment has been minimized.

@edmundoa

This comment has been minimized.

@edmundoa

edmundoa May 26, 2016

Member

Don't know why I didn't do that from the beginning. Using listenTo was actually problematic, as it could try to get some value from the store before the state was initialised.

@@ -0,0 +1 @@
export GraylogClusterOverview from './GraylogClusterOverview';

This comment has been minimized.

@dennisoelkers

dennisoelkers May 26, 2016

Member

This way of reexporting the default export of an ES6 module is not working in some ES6 implementations. Therefore we changed this at some point to follow a form which works in all implementations.

This comment has been minimized.

@edmundoa

edmundoa May 26, 2016

Member

I can't remember that we decided anything about that. Which ES6 implementations don't support that? Anyway, I think we still have some of them around, basically I copied this one from graylog2-web-interface/src/components/times/index.jsx.

This comment has been minimized.

@dennisoelkers

dennisoelkers May 26, 2016

Member

Babel 6 is the one that matters the most for us. I am remembering we talked about this briefly when I changed most of the exports to the format.

This comment has been minimized.

@edmundoa

edmundoa May 26, 2016

Member

I remember IntelliJ didn't like some of them (was highlighting them in red), but don't remember about that. Anyway, I will update this one on the PR and change others I can find in master. In that way we don't copy them around like in this case 👍

return (
<Row className="content">
<Col md={12}>
<h2><i className="fa fa-truck" /> Indexer Failures</h2>

This comment has been minimized.

@dennisoelkers

dennisoelkers May 26, 2016

Member

Why have the glyphicons been removed for each section?

This comment has been minimized.

@edmundoa

edmundoa May 26, 2016

Member

We decided long ago to remove all icons from headers (basically in the UI changes we started in 1.1), as in the end they are mostly random and don't provide any value. The system overview page is the only place that still got some of them, and they weren't even used consistently, so some sections had them, and some not.

Now that I was adding a new section into the page, I decided it was time to get rid of them.

This comment has been minimized.

@dennisoelkers

@dennisoelkers dennisoelkers merged commit 64cee5c into 2.0 May 26, 2016

4 checks passed

ci-server-integration Jenkins build graylog2-server-integration-pr 931 has succeeded
Details
ci-web-linter Jenkins build graylog-pr-linter-check 419 has succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@dennisoelkers dennisoelkers deleted the add-gl-cluster-overview branch May 26, 2016

dennisoelkers added a commit that referenced this pull request May 26, 2016

Add Graylog cluster overview (#2291)
* Add Graylog cluster overview to system overview

* Unify titles in system overview page

- Use same cases
- Remove icons from some of the titles. This was the only place where
  they didn't go away

* Unify the way components load in system overview

Before, some components rendered a loading spinner without drawing a
container, but some didn't.

Now each component shows a container for its data, including the title and
static information. Spinners or content are included into that
container.

* Ensure the node contains a cluster_id property

* Set clusterId and nodeCount in store's state

* Use more standard ES6 reexport

(cherry picked from commit 64cee5c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment