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.

Sign up for GitHub

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

Jump to bottom

Remove environment from transport client #13383

Merged
merged 7 commits into from Sep 8, 2015
Merged

Remove environment from transport client #13383

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1ff49eb
Settings: Remove environment from transport client
rjernst Sep 1, 2015
d19e04f
Change prepareSettingsAndEnvironment to prepareEnvironment
rjernst Sep 8, 2015
92b62c0
Merge branch 'master' into go_away_transport_paths
rjernst Sep 8, 2015
2e63290
Finish removing loadConfigSettings, and fix node startup to use correct
rjernst Sep 8, 2015
86ffa38
Add migration guide note about transport client changes to
rjernst Sep 8, 2015
e218b31
Address PR comments
rjernst Sep 8, 2015
f3fb702
Move logging specific settings setup to log configurator.
rjernst Sep 8, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
View file
Open in desktop
Expand Up @@ -197,7 +197,7 @@ private static void setupLogging(Settings settings, Environment environment) {

private static Tuple<Settings, Environment> initialSettings(boolean foreground) {
Terminal terminal = foreground ? Terminal.DEFAULT : null;
return InternalSettingsPreparer.prepareSettings(EMPTY_SETTINGS, true, terminal);
return InternalSettingsPreparer.prepareSettingsAndEnvironment(EMPTY_SETTINGS, terminal);
Copy link
Contributor

Choose a reason for hiding this comment

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

it might be simpler to just name this InternalSettingsPreparer.prepareEnvironment and return only Environment since it has the settings already as a member then we can spare the tuple and things are less funky?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the only issue with that right now is the last line of the method, which modifies the log file setting to be absolute, after creating the environment.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah I wonder if we should do this first?

Copy link
Member Author

Choose a reason for hiding this comment

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

well it is using the resolved config dir from the environment...

Copy link
Member Author

Choose a reason for hiding this comment

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

but i agree, this is already breaking. i will move it and simplify as you suggested

}

private void start() {
Expand Down
20 changes: 4 additions & 16 deletions core/src/main/java/org/elasticsearch/client/transport/TransportClient.java
View file
Open in desktop
Expand Up @@ -85,7 +85,6 @@ public static class Builder {

private Settings settings = Settings.EMPTY;
private List<Class<? extends Plugin>> pluginClasses = new ArrayList<>();
private boolean loadConfigSettings = true;

/**
* The settings to configure the transport client with.
Expand All @@ -102,15 +101,6 @@ public Builder settings(Settings settings) {
return this;
}

/**
* Should the transport client load file based configuration automatically or not (and rely
* only on the provided settings), defaults to true.
*/
public Builder loadConfigSettings(boolean loadConfigSettings) {
this.loadConfigSettings = loadConfigSettings;
return this;
}

/**
* Add the given plugin to the client when it is created.
*/
Expand All @@ -123,17 +113,16 @@ public Builder addPlugin(Class<? extends Plugin> pluginClass) {
* Builds a new instance of the transport client.
*/
public TransportClient build() {
Tuple<Settings, Environment> tuple = InternalSettingsPreparer.prepareSettings(settings, loadConfigSettings);
Settings settings = settingsBuilder()
Settings settings = InternalSettingsPreparer.prepareSettings(this.settings);
settings = settingsBuilder()
.put(NettyTransport.PING_SCHEDULE, "5s") // enable by default the transport schedule ping interval
.put(tuple.v1())
.put(settings)
.put("network.server", false)
.put("node.client", true)
.put(CLIENT_TYPE_SETTING, CLIENT_TYPE)
.build();
Environment environment = tuple.v2();

PluginsService pluginsService = new PluginsService(settings, tuple.v2(), pluginClasses);
PluginsService pluginsService = new PluginsService(settings, null, pluginClasses);
this.settings = pluginsService.updatedSettings();

Version version = Version.CURRENT;
Expand All @@ -149,7 +138,6 @@ public TransportClient build() {
modules.add(pluginModule);
}
modules.add(new PluginsModule(pluginsService));
modules.add(new EnvironmentModule(environment));
modules.add(new SettingsModule(this.settings));
modules.add(new NetworkModule());
modules.add(new ClusterNameModule(this.settings));
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/elasticsearch/common/cli/CliTool.java
View file
Open in desktop
Expand Up @@ -104,7 +104,7 @@ protected CliTool(CliToolConfig config, Terminal terminal) {
Preconditions.checkArgument(config.cmds().size() != 0, "At least one command must be configured");
this.config = config;
this.terminal = terminal;
Tuple<Settings, Environment> tuple = InternalSettingsPreparer.prepareSettings(EMPTY_SETTINGS, true, terminal);
Tuple<Settings, Environment> tuple = InternalSettingsPreparer.prepareSettingsAndEnvironment(EMPTY_SETTINGS, terminal);
settings = tuple.v1();
env = tuple.v2();
}
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/org/elasticsearch/node/Node.java
View file
Open in desktop
Expand Up @@ -133,7 +133,7 @@ public Node(Settings preparedSettings, boolean loadConfigSettings) {
Node(Settings preparedSettings, boolean loadConfigSettings, Version version, Collection<Class<? extends Plugin>> classpathPlugins) {
final Settings pSettings = settingsBuilder().put(preparedSettings)
.put(Client.CLIENT_TYPE_SETTING, CLIENT_TYPE).build();
Tuple<Settings, Environment> tuple = InternalSettingsPreparer.prepareSettings(pSettings, loadConfigSettings);
Tuple<Settings, Environment> tuple = InternalSettingsPreparer.prepareSettingsAndEnvironment(pSettings, null);
tuple = new Tuple<>(TribeService.processSettings(tuple.v1()), tuple.v2());

ESLogger logger = Loggers.getLogger(Node.class, tuple.v1().get("name"));
Expand All @@ -147,7 +147,7 @@ public Node(Settings preparedSettings, boolean loadConfigSettings) {
env.configFile(), Arrays.toString(env.dataFiles()), env.logsFile(), env.pluginsFile());
}

this.pluginsService = new PluginsService(tuple.v1(), tuple.v2(), classpathPlugins);
this.pluginsService = new PluginsService(tuple.v1(), tuple.v2().pluginsFile(), classpathPlugins);
this.settings = pluginsService.updatedSettings();
// create the environment based on the finalized (processed) view of the settings
this.environment = new Environment(this.settings());
Expand Down