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

Mark transport client as such when instantiating #7552

Conversation

@spinscale
Copy link
Member

commented Sep 3, 2014

This allows plugins to load/inject specific classes, when the client started
is a transport client (compared to being a node client).

@uboness
uboness reviewed Sep 3, 2014
View changes
src/test/java/org/elasticsearch/client/transport/TransportClientTests.java Outdated
TransportClient client = (TransportClient) internalCluster().client();
Settings settings = client.injector.getInstance(Settings.class);
assertThat(settings.getAsBoolean("transport.client", false), is(true));
}

This comment has been minimized.

Copy link
@uboness

uboness Sep 3, 2014

Contributor

maybe add another test the verify that this setting cannot be overridden (so basically, create a transport client with custom setting that sets the transport.client to false and verify that it's still true)?

This comment has been minimized.

Copy link
@javanna

javanna Sep 3, 2014

Member

Also what happens if we set transport.client to true manually within a node client?

This comment has been minimized.

Copy link
@uboness

uboness Sep 3, 2014

Contributor

@javanna very good point... maybe the setting should change to client.type with either transport or node as values, and then we force set this on both the transport client and internal node?

This comment has been minimized.

Copy link
@spinscale

spinscale Sep 3, 2014

Author Member

valid point.. will change that

@spinscale spinscale added the review label Sep 3, 2014
@dadoonet

This comment has been minimized.

Copy link
Member

commented Sep 3, 2014

Ha great! I think @bleskes will like it as well :)

@spinscale spinscale removed the review label Sep 3, 2014
@spinscale spinscale force-pushed the spinscale:enhancement/transport-client-set-settings branch Sep 3, 2014
@spinscale

This comment has been minimized.

Copy link
Member Author

commented Sep 3, 2014

@uboness @javanna added a client.type property, which is either node or transport and cant be changed

@spinscale spinscale added the review label Sep 3, 2014
@javanna

This comment has been minimized.

Copy link
Member

commented Sep 3, 2014

nice @spinscale I like it!

@dadoonet

This comment has been minimized.

Copy link
Member

commented Sep 3, 2014

Could we add a static final String for "transport" and "node" values?

@spinscale spinscale force-pushed the spinscale:enhancement/transport-client-set-settings branch to d5e91dc Sep 3, 2014
@spinscale

This comment has been minimized.

Copy link
Member Author

commented Sep 3, 2014

@uboness moved into the check into InternalNode

@uboness

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2014

LGTM

This allows plugins to load/inject specific classes, when the client started
is a transport client (compared to being a node client).

Closes #7552
@spinscale spinscale force-pushed the spinscale:enhancement/transport-client-set-settings branch to 8b8cc80 Sep 5, 2014
@spinscale spinscale merged commit 8b8cc80 into elastic:master Sep 5, 2014
spinscale added a commit that referenced this pull request Sep 5, 2014
This allows plugins to load/inject specific classes, when the client started
is a transport client (compared to being a node client).

Closes #7552
spinscale added a commit that referenced this pull request Sep 8, 2014
This allows plugins to load/inject specific classes, when the client started
is a transport client (compared to being a node client).

Closes #7552
@jpountz jpountz removed the review label Oct 21, 2014
@clintongormley clintongormley changed the title TransportClient: Mark transport client as such when instantiating Mark transport client as such when instantiating Jun 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.