-
Notifications
You must be signed in to change notification settings - Fork 4.8k
HIVE-26205: Remove the incorrect org.slf4j dependency in kafka-handler #3272
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
Conversation
|
@pvary @deniskuzZ : Could you please review this PR? Project compile failed in my new machine with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This dependency indeed looks wrong. The classes in the kafka-handler module are using the slf4j-api thus this dependency needs to be set at compile scope. As you correctly mentioned the parent pom declares explicitly this dep so removing it from here definitely makes sense.
Additionally there are a bunch of explicit exclusions of slf4j-api from other deps in this file which I believe are redundant. I think those should be removed as well. WDUT?
|
Yes, these exclusions of slf4j-api are also defined in parent pom, can also be removed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I will merge this now.
… (Wechar Yu, reviewed by Stamatis Zampetakis) The classes in the kafka-handler module are using the slf4j-api thus this dependency must be set at compile scope. Currently it is set at test scope which makes the build fail in some recent maven versions ( e.g., 3.8.5). The parent pom declares explicitly slf4j-api at compile scope so removing all the references from kafka-handler/pom.xml is the way to go. Closes apache#3272
… (Wechar Yu, reviewed by Stamatis Zampetakis) The classes in the kafka-handler module are using the slf4j-api thus this dependency must be set at compile scope. Currently it is set at test scope which makes the build fail in some recent maven versions ( e.g., 3.8.5). The parent pom declares explicitly slf4j-api at compile scope so removing all the references from kafka-handler/pom.xml is the way to go. Closes apache#3272
… (Wechar Yu, reviewed by Stamatis Zampetakis) The classes in the kafka-handler module are using the slf4j-api thus this dependency must be set at compile scope. Currently it is set at test scope which makes the build fail in some recent maven versions ( e.g., 3.8.5). The parent pom declares explicitly slf4j-api at compile scope so removing all the references from kafka-handler/pom.xml is the way to go. Closes apache#3272 (cherry picked from commit 74e3f29)
What changes were proposed in this pull request?
Remove the redundant slf4j dependency in kafka-handler module.
Why are the changes needed?
Currently the scope of this dependency is test, which caused the compile errors.
Does this PR introduce any user-facing change?
No
How was this patch tested?
No any more tests.