Repository navigation
Conversation
Contributor
Author
|
This PR is ready for review. Please take a look when you have a chance. Thanks! |
Contributor
|
Thanks for opening this PR @roczei, it is a nice update and another important modernization step. I ran a dependency tree and found a few leftover references to Could you please check if this could/should be fixed as well? |
roczei
force-pushed
the
upgrade_to_slf4j_2
branch
from
October 15, 2026 08:18
996012f to
a2671c4
Compare
Contributor
Author
|
Thanks for the review! I've added the log4j-slf4j-impl exclusion to thriftserver/client/pom.xml. Verified with dependency:tree, it's now completely gone: |
gyogal
reviewed
Oct 15, 2026
| </dependency> | ||
| <dependency> | ||
| <groupId>org.eclipse.jetty</groupId> | ||
| <artifactId>jetty-webapp</artifactId> |
Contributor
There was a problem hiding this comment.
This change seems to solve the issue reported in LIVY-1092 as well.
Contributor
|
Hi @roczei, it seems like there is still an issue with beeline. This is before the update: after the update: Adding |
…-slf4j2-impl) ## What changes were proposed in this pull request? SLF4J 2.0.x replaced the 1.x StaticLoggerBinder lookup with a ServiceLoader-based SLF4JServiceProvider, so the Log4j 2 binding must be swapped from log4j-slf4j-impl (SLF4J 1.x) to log4j-slf4j2-impl (SLF4J 2.x). - pom.xml: slf4j.version 1.7.36 -> 2.0.17 - Swap log4j-slf4j-impl -> log4j-slf4j2-impl in the binding declarations: root dependencyManagement, core, repl (provided), rsc (provided) - thriftserver/client: exclude log4j-slf4j-impl from hive-jdbc (hive-jdbc:3.0.0 still brings log4j-slf4j-impl:2.10.0) and declare log4j-slf4j2-impl explicitly - integration-test: additionally exclude log4j-slf4j2-impl from the test-scoped hive-jdbc (next to the existing log4j-slf4j-impl exclusion) - thriftserver/server: replace the jetty-runner uber-jar (which bundles a shaded SLF4J 1.x that shadowed slf4j-api 2.x and caused a "Failed to load class org.slf4j.impl.StaticLoggerBinder" NOP fallback at server startup) with the individual jetty-server, jetty-servlet and jetty-webapp modules, and drop the now-obsolete jetty-util/jetty-xml/jetty-webapp excludeArtifactIds that only existed to avoid duplicating jetty-runner's bundled copies. The existing log4j-slf4j-impl exclusion on hive-jdbc is kept. No Log4j bump needed (already on 2.26.0, >= the 2.23.0 minimum for log4j-slf4j2-impl). Livy has no org.slf4j.impl / StaticLoggerBinder usage, so no source changes are required. JIRA: https://issues.apache.org/jira/browse/LIVY-1090 ## How was this patch tested? Unit tests. - core/src/test/scala/org/apache/livy/LoggingSuite.scala: verifies a single SLF4JServiceProvider is registered and resolves to Log4j 2 (org.apache.logging.slf4j), and that messages logged through the Logging trait are routed to Log4j 2. Passes 2/2 on Scala 2.13 and test-compiles under Scala 2.12. - thriftserver/server/src/test/scala/org/apache/livy/thriftserver/TestThriftServerLogging.scala: asserts exactly one org.slf4j.LoggerFactory is on the classpath (fails if a shaded SLF4J such as jetty-runner is reintroduced) and that SLF4J binds to Log4j 2 rather than falling back to the NOP logger. Also verified via mvn dependency:tree on core, repl, rsc, server, and thriftserver/server: slf4j-api resolves to 2.0.17, log4j-slf4j2-impl is present, and no log4j-slf4j-impl remains on the compile/runtime classpath (including under hive-jdbc). Started livy-server locally and confirmed the StaticLoggerBinder NOP warning is gone and logging works. ## Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 4.8) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
roczei
force-pushed
the
upgrade_to_slf4j_2
branch
from
October 16, 2026 16:06
a2671c4 to
5e951c1
Compare
Contributor
Author
Good catch, thanks! I have just fixed it: |
This branch has not been deployed
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
2 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.
What changes were proposed in this pull request?
SLF4J 2.0.x replaced the 1.x StaticLoggerBinder lookup with a ServiceLoader-based SLF4JServiceProvider, so the Log4j 2 binding must be swapped from log4j-slf4j-impl (SLF4J 1.x) to log4j-slf4j2-impl (SLF4J 2.x).
No Log4j bump needed (already on 2.26.0, >= the 2.23.0 minimum for log4j-slf4j2-impl). Livy has no org.slf4j.impl / StaticLoggerBinder usage, so no source changes are required.
JIRA: https://issues.apache.org/jira/browse/LIVY-1090
How was this patch tested?
Unit tests.
Also verified via mvn dependency:tree on core, repl, rsc, server, and thriftserver/server: slf4j-api resolves to 2.0.17, log4j-slf4j2-impl is present, and no log4j-slf4j-impl remains on the compile/runtime classpath (including under hive-jdbc). Started livy-server locally and confirmed the StaticLoggerBinder NOP warning is gone and logging works.
Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 4.8)