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 _cat/nodeattrs API #12534

Merged
merged 1 commit into from Jul 29, 2015
Merged

Add _cat/nodeattrs API #12534

merged 1 commit into from Jul 29, 2015

Conversation

bookshelfdave
Copy link
Contributor

This provides a _cat/nodeattrs API call, which presents custom node attributes in a denormalized table.

This was added in addition to the _cat/nodes endpoint as we don't want to create dynamic columns for each user defined attribute in _cat/nodes.

Closes #8000

cc @drewr

@nik9000
Copy link
Member

nik9000 commented Jul 29, 2015

Looks good to me. I'm surprised it takes three requests to get that but I don't really mind it.

table.addCell("ip", "alias:i;desc:ip address");
table.addCell("port", "default:false;alias:po;desc:bound transport port");
table.addCell("attr.name");
table.addCell("attr.value");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I'm seeing these, I wonder if the default names should be attr and value. We could add aliases if we need longer. Since it's such a small API it's probably fine with the short versions.

@drewr
Copy link
Contributor

drewr commented Jul 29, 2015

Other than small comment, LGTM, thanks @metadave!

@nik9000 That's the beauty of the cat API, it slices across the stats and info APIs internally so you don't have to! 😹

@bookshelfdave
Copy link
Contributor Author

Updated with @drewr's suggestions. How do I go about getting a doc for http://www.elastic.co/guide/en/elasticsearch/reference/master/cat-nodeattrs.html ?

@nik9000
Copy link
Member

nik9000 commented Jul 29, 2015

How do I go about getting a doc for http://www.elastic.co/guide/en/elasticsearch/reference/master/cat-nodeattrs.html ?

Make one in docs/reference/cat I believe.

@bookshelfdave
Copy link
Contributor Author

thanks @nik9000

@nik9000
Copy link
Member

nik9000 commented Jul 29, 2015

Do it as part of this pull request if you can! Its better to commit the docs at the same time.

This has documentation on how to build them: https://github.com/elastic/docs

@bookshelfdave
Copy link
Contributor Author

Updated w/ asciidoc

@nik9000
Copy link
Member

nik9000 commented Jul 29, 2015

Do you need to add a link in the cat.asciidoc?

@bookshelfdave
Copy link
Contributor Author

yes, indeed I do.

clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
clusterStateRequest.masterNodeTimeout(request.paramAsTime("master_timeout", clusterStateRequest.masterNodeTimeout()));

client.admin().cluster().state(clusterStateRequest, new RestActionListener<ClusterStateResponse>(channel) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we want this optimization, but we could always run these three requests in parallel, putting the responses in three AtomicReferences and using three CountDownLatchs to wait until all three complete.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I implemented the early cat APIs that way and @kimchy wanted to nest instead. If the cluster's busy then you only have one dangling request.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That works for me!

@bookshelfdave
Copy link
Contributor Author

shall I squash and merge?

@dakrone
Copy link
Member

dakrone commented Jul 29, 2015

Yep, LGTM, can you also add the v2.0.0 tag?

This provides a _cat/nodeattrs API call, which presents
custom node attributes in a denormalized table.

Closes #8000
bookshelfdave added a commit that referenced this pull request Jul 29, 2015
@bookshelfdave bookshelfdave merged commit 2d2f21e into elastic:master Jul 29, 2015
@clintongormley clintongormley removed the good first issue low hanging fruit label Jul 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants