-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Move the CLI into its own subproject #27114
Changes from 18 commits
c4308ce
c6cc762
73745ea
7eda8e2
12dd213
e248c16
9c61c51
037259c
dcdaaf8
9cd8a89
62612ac
db9980e
87448d3
b1cfecb
1e0a98c
c179020
75db462
21ed445
1fb5f1b
94bbb74
905546b
2ead8d6
529f7f5
2c366c1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
/* | ||
* Licensed to Elasticsearch under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
import org.elasticsearch.gradle.precommit.PrecommitTasks | ||
|
||
apply plugin: 'elasticsearch.build' | ||
|
||
archivesBaseName = 'elasticsearch-cli' | ||
|
||
dependencies { | ||
compile 'net.sf.jopt-simple:jopt-simple:5.0.2' | ||
} | ||
|
||
test.enabled = false | ||
// Since CLI does not depend on :core, it cannot run the jarHell task | ||
jarHell.enabled = false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should refactor out the JAR hell check too, as there are other places where we will want to run JAR hell checks that will not depend on core. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as part of this PR or just as a separate issue @jasontedor ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @hub-cap Separate is fine. |
||
|
||
forbiddenApisMain { | ||
signaturesURLs = [PrecommitTasks.getResource('/forbidden/jdk-signatures.txt')] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,11 +23,6 @@ | |
import joptsimple.OptionParser; | ||
import joptsimple.OptionSet; | ||
import joptsimple.OptionSpec; | ||
import org.apache.logging.log4j.Level; | ||
import org.apache.lucene.util.SetOnce; | ||
import org.elasticsearch.common.SuppressForbidden; | ||
import org.elasticsearch.common.logging.LogConfigurator; | ||
import org.elasticsearch.common.settings.Settings; | ||
|
||
import java.io.Closeable; | ||
import java.io.IOException; | ||
|
@@ -55,12 +50,13 @@ public Command(String description) { | |
this.description = description; | ||
} | ||
|
||
final SetOnce<Thread> shutdownHookThread = new SetOnce<>(); | ||
private Thread shutdownHookThread; | ||
|
||
/** Parses options for this command from args and executes it. */ | ||
public final int main(String[] args, Terminal terminal) throws Exception { | ||
if (addShutdownHook()) { | ||
shutdownHookThread.set(new Thread(() -> { | ||
|
||
shutdownHookThread = new Thread(() -> { | ||
try { | ||
this.close(); | ||
} catch (final IOException e) { | ||
|
@@ -75,16 +71,11 @@ public final int main(String[] args, Terminal terminal) throws Exception { | |
throw new AssertionError(impossible); | ||
} | ||
} | ||
})); | ||
Runtime.getRuntime().addShutdownHook(shutdownHookThread.get()); | ||
}); | ||
Runtime.getRuntime().addShutdownHook(shutdownHookThread); | ||
} | ||
|
||
if (shouldConfigureLoggingWithoutConfig()) { | ||
// initialize default for es.logger.level because we will not read the log4j2.properties | ||
final String loggerLevel = System.getProperty("es.logger.level", Level.INFO.name()); | ||
final Settings settings = Settings.builder().put("logger.level", loggerLevel).build(); | ||
LogConfigurator.configureWithoutConfig(settings); | ||
} | ||
beforeExecute(); | ||
|
||
try { | ||
mainWithoutErrorHandling(args, terminal); | ||
|
@@ -103,14 +94,10 @@ public final int main(String[] args, Terminal terminal) throws Exception { | |
} | ||
|
||
/** | ||
* Indicate whether or not logging should be configured without reading a log4j2.properties. Most commands should do this because we do | ||
* not configure logging for CLI tools. Only commands that configure logging on their own should not do this. | ||
* | ||
* @return true if logging should be configured without reading a log4j2.properties file | ||
* Setup method to be executed before parsing or execution of the command being run. Any exceptions thrown by the | ||
* method will not be cleanly caught by the parser. | ||
*/ | ||
protected boolean shouldConfigureLoggingWithoutConfig() { | ||
return true; | ||
} | ||
protected void beforeExecute() throws Exception {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does this declare a checked exception? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cruft from the refactor. itll be gone next push. |
||
|
||
/** | ||
* Executes the command, but all errors are thrown. | ||
|
@@ -166,6 +153,11 @@ protected boolean addShutdownHook() { | |
return true; | ||
} | ||
|
||
/** Gets the shutdown hook thread if it exists **/ | ||
public Thread getShutdownHookThread() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this needs to be public, only package-private? |
||
return shutdownHookThread; | ||
} | ||
|
||
@Override | ||
public void close() throws IOException { | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
/* | ||
* Licensed to Elasticsearch under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
package org.elasticsearch.cli; | ||
|
||
import java.lang.annotation.ElementType; | ||
import java.lang.annotation.Retention; | ||
import java.lang.annotation.RetentionPolicy; | ||
import java.lang.annotation.Target; | ||
|
||
/** | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you spare a line of whitespace? |
||
* Annotation to suppress forbidden-apis errors inside a whole class, a method, or a field. | ||
*/ | ||
@Retention(RetentionPolicy.CLASS) | ||
@Target({ ElementType.CONSTRUCTOR, ElementType.FIELD, ElementType.METHOD, ElementType.TYPE }) | ||
public @interface SuppressForbidden { | ||
String reason(); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we actually need to publish this? If we first move the ES cli under a
distribution/tools
project, then this could could be something likedistribution/tools/base-cli
?