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

Test: Allow CliTool to write out stacktraces #7222

Merged
merged 1 commit into from Aug 12, 2014

Conversation

Projects
None yet
4 participants
@spinscale
Copy link
Member

spinscale commented Aug 11, 2014

In order to have the possibility of debugging on the command line, the user
now can either set the ES_CLI_DEBUG environment variable or at es.cli.debug system
property which results in stack traces being written to stdout.

@spinscale spinscale added the review label Aug 11, 2014

@uboness

View changes

src/main/java/org/elasticsearch/common/cli/Terminal.java Outdated
@@ -108,6 +113,14 @@ public void printError(String msg, Object... args) {
println(Verbosity.SILENT, "ERROR: " + msg, args);
}

public void printError(Throwable t) {
printError("%s", t.getMessage());
boolean isDebugEnabled = "true".equals(System.getProperty(DEBUG_SYSTEM_PROPERTY, System.getenv(DEBUG_ENV_VAR)));

This comment has been minimized.

Copy link
@uboness

uboness Aug 11, 2014

Contributor

why not pull this out to a static var called debugMode?

This comment has been minimized.

Copy link
@uboness

uboness Aug 11, 2014

Contributor

otherwise, if you want it for testing, it can be done once in the ctor

@uboness

View changes

src/main/java/org/elasticsearch/common/cli/Terminal.java Outdated
@@ -94,6 +97,8 @@ public void println(Verbosity verbosity) {
println(verbosity, "");
}

public abstract void printErrorInternal(Throwable t);

This comment has been minimized.

Copy link
@uboness

uboness Aug 11, 2014

Contributor

can be protected and would call it printStackTrace

@uboness

View changes

src/main/java/org/elasticsearch/common/cli/Terminal.java Outdated
@@ -29,6 +29,9 @@
*/
public abstract class Terminal {

public static final String DEBUG_SYSTEM_PROPERTY = "es.cli.debug";
public static final String DEBUG_ENV_VAR = "ES_CLI_DEBUG";

This comment has been minimized.

Copy link
@uboness

uboness Aug 11, 2014

Contributor

not sure if we really need an env var for it... I think system property is enough?

@uboness

This comment has been minimized.

Copy link
Contributor

uboness commented Aug 11, 2014

left a few comments, otherwise LGTM

@jpountz jpountz removed the review label Aug 12, 2014

@spinscale

This comment has been minimized.

Copy link
Member Author

spinscale commented Aug 12, 2014

@uboness incorporated all of your review comments, were all good and valid. thx!

@uboness

This comment has been minimized.

Copy link
Contributor

uboness commented Aug 12, 2014

LGTM

Test: Allow CliTool to write out stacktraces
In order to have the possibility of debugging on the command line, the user
now can either set the es.cli.debug system property
which results in stack traces being written to to the terminal.

Closes #7222

@spinscale spinscale merged commit e689a0a into elastic:master Aug 12, 2014

spinscale added a commit that referenced this pull request Aug 12, 2014

Test: Allow CliTool to write out stacktraces
In order to have the possibility of debugging on the command line, the user
now can either set the es.cli.debug system property
which results in stack traces being written to to the terminal.

Closes #7222

spinscale added a commit that referenced this pull request Sep 8, 2014

Test: Allow CliTool to write out stacktraces
In order to have the possibility of debugging on the command line, the user
now can either set the es.cli.debug system property
which results in stack traces being written to to the terminal.

Closes #7222

@clintongormley clintongormley added >test and removed >enhancement labels Jun 7, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.