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

Ensure logging configuration is loaded in plugin manager #12081

Merged

Conversation

spinscale
Copy link
Contributor

This prevents log4j warnings printed out, when installing a plugin
due to the JarHell class using an ESLogger.

Closes #12064

@dadoonet
Copy link
Member

dadoonet commented Jul 7, 2015

LGTM

@dadoonet
Copy link
Member

dadoonet commented Jul 7, 2015

I think we also have this issue when running REST tests in plugins BTW. Or when running tests from IntelliJ in plugins.

@spinscale
Copy link
Contributor Author

@dadoonet can you show me how to reproduce that? RestTests are running fine for me, with logging...also in intellij

@rmuir
Copy link
Contributor

rmuir commented Jul 7, 2015

Looks good. I think @dadoonet refers to the new rest tests mechanism that runs in mvn verify. You can run it with mvn verify -Dskip.unit.tests to see. This mechanism unzips the elasticsearch.zip and installs the plugin with bin/plugin, before running rest tests against that external cluster, so thats why you see it there.

@rmuir
Copy link
Contributor

rmuir commented Jul 7, 2015

and also thanks for tracking this one down :)

@spinscale
Copy link
Contributor Author

ah, so in order to not have the messages occur in mvn verify, I had to run mvn install before - which is obvious after reading the ant file. If we want to have run this as part of the release, we just shouldnt forget to run that, as we dont do know. will get it in, in a sec

This prevents log4j warnings printed out, when installing a plugin
due to the JarHell class using an ESLogger.

Closes elastic#12064
@spinscale spinscale force-pushed the 1507-issue-12064-fix-log4j-config branch from a47dd8e to 21b4f9b Compare July 7, 2015 12:56
@spinscale spinscale merged commit 21b4f9b into elastic:master Jul 7, 2015
@kevinkluge kevinkluge removed the review label Jul 7, 2015
@rmuir
Copy link
Contributor

rmuir commented Jul 7, 2015

we can remove the mvn install requirement so that it uses the "reactor" classpath in the future. it just means using a different property set by the maven build itself from that ant logic.

@clintongormley clintongormley changed the title Plugins: Ensure logging configuration is loaded in plugin manager Ensure logging configuration is loaded in plugin manager Aug 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants