Skip to content

[tribe] Creates single interface to retrieve clusters#9266

Merged
tylersmalley merged 3 commits intoelastic:tribe-node-supportfrom
tylersmalley:tribe-getcluster
Dec 5, 2016
Merged

[tribe] Creates single interface to retrieve clusters#9266
tylersmalley merged 3 commits intoelastic:tribe-node-supportfrom
tylersmalley:tribe-getcluster

Conversation

@tylersmalley
Copy link
Member

@tylersmalley tylersmalley commented Nov 30, 2016

Previously we had been adding interfaces directly to the Hapi plugin (client, ElasticsearchClientLogging, createClient, callWithRequestFactory, callWithRequest). Since we are now further expanding on that functionality but adding another cluster (Admin/Data), it's time to clean up the interface.

This adds getCluster and createCluster to server.plugins.elasticsearch. getCluster now returns a cluster object which contains the config (including log), createClient, callWithRequestFactory and callWithRequest.

For #9132

@tylersmalley tylersmalley added the Team:Operations Kibana-Operations Team label Nov 30, 2016
@tylersmalley tylersmalley changed the title [tribe] Creates single interface to retreive clusters [tribe] Creates single interface to retrieve clusters Nov 30, 2016
@jbudz jbudz self-assigned this Nov 30, 2016
Copy link
Member Author

Choose a reason for hiding this comment

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

Not set on adding, but needed to make it configurable for the tests to complete faster. I have heard of users manually changing the constant in health_check.js.

Copy link
Contributor

@jbudz jbudz left a comment

Choose a reason for hiding this comment

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

Thoughts on renaming callAsKibanaUser --> callAsKibanaServerUser or callWithServerAuthentication/something along those lines to avoid any ambiguity? In the case of not using the security plugin, the default user isn't necessarily kibana.

Copy link
Contributor

Choose a reason for hiding this comment

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

is this defined?

Copy link
Member Author

Choose a reason for hiding this comment

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

callAsKibanaUser? Yeah, it's being passed as callAdminAsKibanaUser which is defined on line 20. I can see how that is confusing. Thoughts on what we can rename it to? caller?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah okay, got it.

Copy link
Contributor

Choose a reason for hiding this comment

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

here too

@jbudz
Copy link
Contributor

jbudz commented Nov 30, 2016

jenkins, test this

@jbudz
Copy link
Contributor

jbudz commented Nov 30, 2016

Test failures seem to be legitimate

Previous we had been adding interfaces directly to the Hapi plugin (client, ElasticsearchClientLogging, createClient, callWithRequestFactory, callWithRequest). Since we are now further expanding on that functionality but adding another cluster (Admin/Data), it's time to clean up the interface.

This adds getCluster and createCluster to server.plugins.elasticsearch. getCluster now returns a cluster object which contains the config (including log), createClient, callWithRequestFactory and callWithRequest.

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@tylersmalley
Copy link
Member Author

@jbudz

Thoughts on renaming callAsKibanaUser --> callAsKibanaServerUser or callWithServerAuthentication/something along those lines to avoid any ambiguity? In the case of not using the security plugin, the default user isn't necessarily kibana.

What about calling it call? So, getCluster('admin').call(). callWithRequest is really just the same, but takes the request into context.

Thoughts?

@tylersmalley
Copy link
Member Author

@jbudz, tests are now passing.

@jbudz
Copy link
Contributor

jbudz commented Dec 1, 2016

@tylersmalley call may be confusing with Function.call

}

if (!api) {
throw new Error(`callWithRequest called with an invalid endpoint: ${endpoint}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

callWithRequest = callAsKibanaUser?

@tylersmalley
Copy link
Member Author

Ah, I spaced on that. We could do send and sendWithRequest.

@tylersmalley tylersmalley force-pushed the tribe-getcluster branch 3 times, most recently from c2b16b9 to a50eaa1 Compare December 5, 2016 15:35
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@tylersmalley tylersmalley merged commit adf4a82 into elastic:tribe-node-support Dec 5, 2016
@tylersmalley tylersmalley deleted the tribe-getcluster branch December 5, 2016 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Team:Operations Kibana-Operations Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants