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 the cluster name to the "/" endpoint #7524

Closed
wants to merge 2 commits into from

Conversation

bleskes
Copy link
Contributor

@bleskes bleskes commented Aug 31, 2014

The root endpoint returns basic information about this node, like it's name and ES version etc. The cluster name is an important information that belongs in that list.

The root endpoint returns basic information about this node, like it's name and ES version etc. The cluster name is an important information that belongs in that list.
super(settings, client);
this.version = version;
controller.registerHandler(GET, "/", this);
controller.registerHandler(HEAD, "/", this);
this.clusterName = clusterName;
Copy link
Member

Choose a reason for hiding this comment

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

Unlikely to matter, but you should put this assignment above the handler registration since it hands out a reference to an incomplete object that could cause trouble.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough. Pushed another commit with this changed.

@spinscale
Copy link
Contributor

LGTM, useful tiny little piece!

@bleskes bleskes closed this in 0e6bb1f Sep 1, 2014
bleskes added a commit that referenced this pull request Sep 1, 2014
The root endpoint returns basic information about this node, like it's name and ES version etc. The cluster name is an important information that belongs in that list.

Closes #7524
@bleskes bleskes deleted the cluster_name_in_root branch September 1, 2014 08:07
bleskes added a commit that referenced this pull request Sep 8, 2014
The root endpoint returns basic information about this node, like it's name and ES version etc. The cluster name is an important information that belongs in that list.

Closes #7524
@clintongormley clintongormley changed the title [Rest] Add the cluster name to the "/" endpoint REST: Add the cluster name to the "/" endpoint Sep 8, 2014
@clintongormley clintongormley changed the title REST: Add the cluster name to the "/" endpoint REST API: Add the cluster name to the "/" endpoint Sep 11, 2014
@jpountz jpountz removed the review label Oct 21, 2014
@clintongormley clintongormley added the :Core/Infra/REST API REST infrastructure and utilities label Jun 7, 2015
@clintongormley clintongormley changed the title REST API: Add the cluster name to the "/" endpoint Add the cluster name to the "/" endpoint Jun 7, 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