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
Contributor

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

TransportClient client = (TransportClient) internalCluster().client();
Settings settings = client.injector.getInstance(Settings.class);
assertThat(settings.getAsBoolean("transport.client", false), is(true));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

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)?

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

valid point.. will change that

@dadoonet
Copy link
Member

dadoonet 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 enhancement/transport-client-set-settings branch from 1e111f1 to cd5cabd Compare September 3, 2014 12:06
@spinscale
Copy link
Contributor Author

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

@javanna
Copy link
Member

javanna commented Sep 3, 2014

nice @spinscale I like it!

@dadoonet
Copy link
Member

dadoonet commented Sep 3, 2014

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

@spinscale spinscale force-pushed the enhancement/transport-client-set-settings branch from cd5cabd to d5e91dc Compare September 3, 2014 16:40
@spinscale
Copy link
Contributor Author

@uboness moved into the check into InternalNode

@uboness
Copy link
Contributor

uboness 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 elastic#7552
@spinscale spinscale force-pushed the enhancement/transport-client-set-settings branch from 6f6144b to 8b8cc80 Compare September 5, 2014 13:16
@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 added the :Core/Infra/Transport API Transport client API label Jun 7, 2015
@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
Development

Successfully merging this pull request may close these issues.

None yet

6 participants