Remove log4j dependency from elasticsearch-core#28705
Merged
dakrone merged 3 commits intoelastic:masterfrom Feb 20, 2018
Merged
Remove log4j dependency from elasticsearch-core#28705dakrone merged 3 commits intoelastic:masterfrom
dakrone merged 3 commits intoelastic:masterfrom
Conversation
This removes the log4j dependency from our elasticsearch-core project. It was originally necessary only for our jar classpath checking. It is now replaced by a `Consumer<String>` so that the es-core dependency doesn't have external dependencies. The parts of elastic#28191 which were moved in conjunction (like `ESLoggerFactory` and `Loggers`) have been moved back where appropriate, since they are not required in the core jar. This is tangentially related to elastic#28504
Member
Author
|
I also tested locally that this doesn't break the output from the gradle task that invokes the And an error still causes output: |
jasontedor
reviewed
Feb 16, 2018
| * @throws IllegalStateException if jar hell was found | ||
| */ | ||
| public static void checkJarHell() throws IOException, URISyntaxException { | ||
| public static void checkJarHell(Consumer<String> output) throws IOException, URISyntaxException { |
Member
There was a problem hiding this comment.
Can you update the Javadoc to explain this parameter?
Member
Author
There was a problem hiding this comment.
Certainly, I've pushed a commit that does this
jasontedor
approved these changes
Feb 16, 2018
Member
jasontedor
left a comment
There was a problem hiding this comment.
A few more minors, no need for another round.
| /** | ||
| * Checks the set of URLs for duplicate classes | ||
| * @param urls A set of URLs from the classpath to be checked for conflicting jars | ||
| * @param output A {@code String} {@link Consumer} to which debug output will be sent |
Member
There was a problem hiding this comment.
I guess String should be @link too?
|
|
||
| /** | ||
| * Checks the current classpath for duplicate classes | ||
| * @param output A {@code String} {@link Consumer} to which debug output will be sent |
| } | ||
| output.accept("java.class.path: " + System.getProperty("java.class.path")); | ||
| output.accept("sun.boot.class.path: " + System.getProperty("sun.boot.class.path")); | ||
| if (loader instanceof URLClassLoader ) { |
Member
There was a problem hiding this comment.
Nit, and I know it was there, but there's an extra space between URLClassLoader and ).
dakrone
added a commit
that referenced
this pull request
Feb 20, 2018
* Remove log4j dependency from elasticsearch-core This removes the log4j dependency from our elasticsearch-core project. It was originally necessary only for our jar classpath checking. It is now replaced by a `Consumer<String>` so that the es-core dependency doesn't have external dependencies. The parts of #28191 which were moved in conjunction (like `ESLoggerFactory` and `Loggers`) have been moved back where appropriate, since they are not required in the core jar. This is tangentially related to #28504 * Add javadocs for `output` parameter * Change @code to @link
sebasjm
pushed a commit
to sebasjm/elasticsearch
that referenced
this pull request
Mar 10, 2018
* Remove log4j dependency from elasticsearch-core This removes the log4j dependency from our elasticsearch-core project. It was originally necessary only for our jar classpath checking. It is now replaced by a `Consumer<String>` so that the es-core dependency doesn't have external dependencies. The parts of elastic#28191 which were moved in conjunction (like `ESLoggerFactory` and `Loggers`) have been moved back where appropriate, since they are not required in the core jar. This is tangentially related to elastic#28504 * Add javadocs for `output` parameter * Change @code to @link
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This removes the log4j dependency from our elasticsearch-core project. It was
originally necessary only for our jar classpath checking. It is now replaced by
a
Consumer<String>so that the es-core dependency doesn't have externaldependencies.
The parts of #28191 which were moved in conjunction (like
ESLoggerFactoryandLoggers) have been moved back where appropriate, since they are not requiredin the core jar.
This is tangentially related to #28504