mneethiraj commented on code in PR #628:
URL: https://github.com/apache/ranger/pull/628#discussion_r2279856099
##########
ranger-elasticsearch-plugin-shim/pom.xml:
##########
@@ -55,12 +55,25 @@
<artifactId>commons-lang</artifactId>
<version>${commons.lang.version}</version>
</dependency>
-
<dependency>
- <groupId>org.slf4j</groupId>
- <artifactId>slf4j-log4j12</artifactId>
- <version>${slf4j-api.version}</version>
- <scope>runtime</scope>
+ <groupId>org.apache.logging.log4j</groupId>
Review Comment:
`ranger-elasticsearch-plugin-shim.jar` shouldn't require log4j
implementation libraries; Sources in this library only refer to
`org.slf4j.Logger` and `org.slf4j.LoggerFactory`; so depdency on `slf4j-api`
should be the enough. The implementation libraries, log4j for Elasticsearch,
should be picked up at runtime from Elasticsearch classpath. It shouldn't be
necessary to package them with Ranger plugin.
Also, looking at the stack trace in RANGER-3696, it seems Elastisearch
already has class isolation in place for plugins (so that libraries used in the
plugin are not visible to Elasticsearch). If this is true, then I suggest
getting rid of ranger-elasticsearch-plugin-shim library altogether.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@ranger.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
