You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@orc.apache.org by "zabetak (via GitHub)" <gi...@apache.org> on 2023/02/08 16:19:01 UTC

[GitHub] [orc] zabetak opened a new pull request, #1403: ORC-1371: Remove unsupported SLF4J bindings from classpath

zabetak opened a new pull request, #1403:
URL: https://github.com/apache/orc/pull/1403

   ### What changes were proposed in this pull request?
   * Exclude `slf4j-log4j12` from Hadoop dependencies
   * Exclude `logback-classic` from Zookeeper
   
   ### Why are the changes needed?
   Running `./mvnw clean install` shows a lot of warnings of the following form:
   ```
   SLF4J: Class path contains SLF4J bindings targeting slf4j-api versions 1.7.x or earlier.
   SLF4J: Ignoring binding found at [jar:file:/home/stamatis/.m2/repository/org/slf4j/slf4j-log4j12/1.7.10/slf4j-log4j12-1.7.10.jar!/org/slf4j/impl/StaticLoggerBinder.class]
   SLF4J: Ignoring binding found at [jar:file:/home/stamatis/.m2/repository/ch/qos/logback/logback-classic/1.2.10/logback-classic-1.2.10.jar!/org/slf4j/impl/StaticLoggerBinder.class]
   SLF4J: See https://www.slf4j.org/codes.html#ignoredBindings for an explanation.
   ```
   
   The problem is that slf4j-log4j12-1.7.10.jar and logback-classic-1.2.10.jar are in the classpath but are incompatible with the current version of slf4j-api that ORC uses.
   
   ### How was this patch tested?
   `./mvnw clean install 2>&1 | grep "Class path contains SLF4J bindings" | wc -l`
   
   Before: 74
   After: 0
   


-- 
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: issues-unsubscribe@orc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [orc] dongjoon-hyun commented on pull request #1403: ORC-1371: Remove unsupported SLF4J bindings from classpath

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #1403:
URL: https://github.com/apache/orc/pull/1403#issuecomment-1423280137

   Also, cc @williamhyun 


-- 
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: issues-unsubscribe@orc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [orc] zabetak commented on a diff in pull request #1403: ORC-1371: Remove unsupported SLF4J bindings from classpath

Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on code in PR #1403:
URL: https://github.com/apache/orc/pull/1403#discussion_r1101212637


##########
java/mapreduce/pom.xml:
##########
@@ -69,7 +69,6 @@
     <dependency>
       <groupId>org.apache.hadoop</groupId>
       <artifactId>hadoop-mapreduce-client-jobclient</artifactId>
-      <version>${min.hadoop.version}</version>

Review Comment:
   We need to add an exclusion for `hadoop-mapreduce-client-jobclient`. This can be done here or in the `dependencyManagement` section in the parent `pom.xml`.
   
   If we do it here then if someone in the future adds `hadoop-mapreduce-client-jobclient` in another module without copying the exclusion things will be broken again. The same can happen if `hadoop-mapreduce-client-jobclient` appears transitively from somewhere else. 
   
   On the other hand, if we add `hadoop-mapreduce-client-jobclient` in the `dependencyManagement` we are sure that the problems mentioned previously will not happen.
   
   In general it is a good practice to add all dependencies in the [dependency-management section|https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#dependency-management] so I am always in favor of doing that.
   
   Opinions can differ though, so I am happy to make the changes if you think that it would be better to just add the exclusion here.



-- 
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: issues-unsubscribe@orc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [orc] zabetak commented on a diff in pull request #1403: ORC-1371: Remove unsupported SLF4J bindings from classpath

Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on code in PR #1403:
URL: https://github.com/apache/orc/pull/1403#discussion_r1101212637


##########
java/mapreduce/pom.xml:
##########
@@ -69,7 +69,6 @@
     <dependency>
       <groupId>org.apache.hadoop</groupId>
       <artifactId>hadoop-mapreduce-client-jobclient</artifactId>
-      <version>${min.hadoop.version}</version>

Review Comment:
   We need to add an exclusion for `hadoop-mapreduce-client-jobclient`. This can be done here or in the `dependencyManagement` section in the parent `pom.xml`.
   
   If we do it here then if someone in the future adds `hadoop-mapreduce-client-jobclient` in another module without copying the exclusion things will be broken again. The same can happen if `hadoop-mapreduce-client-jobclient` appears transitively from somewhere else. 
   
   On the other hand, if we add `hadoop-mapreduce-client-jobclient` in the `dependencyManagement` we are sure that the problems mentioned previously will not happen.
   
   In general it is a good practice to add all dependencies in the [dependency-management](https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#dependency-management) section so I am always in favor of doing that.
   
   Opinions can differ though, so I am happy to make the changes if you think that it would be better to just add the exclusion here.



-- 
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: issues-unsubscribe@orc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [orc] dongjoon-hyun closed pull request #1403: ORC-1371: Remove unsupported SLF4J bindings from classpath

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #1403: ORC-1371: Remove unsupported SLF4J bindings from classpath
URL: https://github.com/apache/orc/pull/1403


-- 
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: issues-unsubscribe@orc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [orc] zabetak commented on pull request #1403: ORC-1371: Remove unsupported SLF4J bindings from classpath

Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on PR #1403:
URL: https://github.com/apache/orc/pull/1403#issuecomment-1423911095

   @deshanxiao See my comment here: https://github.com/apache/orc/pull/1403/files#r1101212637


-- 
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: issues-unsubscribe@orc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [orc] dongjoon-hyun commented on pull request #1403: ORC-1371: Remove unsupported SLF4J bindings from classpath

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #1403:
URL: https://github.com/apache/orc/pull/1403#issuecomment-1426582870

   Merged to main branch for Apache ORC 1.9.0.
   
   Welcome to the Apache ORC community. I added you to the Apache ORC contributor group and assigned ORC-1371 to you, @zabetak . 


-- 
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: issues-unsubscribe@orc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [orc] dongjoon-hyun commented on a diff in pull request #1403: ORC-1371: Remove unsupported SLF4J bindings from classpath

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #1403:
URL: https://github.com/apache/orc/pull/1403#discussion_r1100712351


##########
java/mapreduce/pom.xml:
##########
@@ -69,7 +69,6 @@
     <dependency>
       <groupId>org.apache.hadoop</groupId>
       <artifactId>hadoop-mapreduce-client-jobclient</artifactId>
-      <version>${min.hadoop.version}</version>

Review Comment:
   Is this inevitably needed in this `SLF4J binding` PR? Otherwise, shall we move out from this PR?



-- 
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: issues-unsubscribe@orc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org