[SPARK-33495][BUILD] Remove commons-logging.jar's dependency #30470
Conversation
| <groupId>commons-logging</groupId> | ||
| <artifactId>commons-logging</artifactId> | ||
| <!-- Hive uses commons-logging 1.1.3 from 0.13 to 1.2 --> | ||
| <version>1.1.3</version> |
There was a problem hiding this comment.
Could you manual test if we can connect Hive 0.13 - Hive-1.2 after removing commons-logging?
There was a problem hiding this comment.
Could you manual test if we can connect Hive 0.13 - Hive-1.2 after removing commons-logging?
Yea, our env is hive 1.2.1, I will test this later.
There was a problem hiding this comment.
Could you manual test if we can connect Hive 0.13 - Hive-1.2 after removing commons-logging?
Hive need 0.13~ 1.2 need commons-logging. but jcl-over-slf4j can replace it. For different usage of hive jars:
- builtin: not supported yet.
- maven: will download commons-logging-1.1.13, but only this jar in hive client 's classLoader, not a problem since
jcl-over-slf4jandcommons-loggingnot coexistence. - path/classpath: user's behavior, we can't control, but better let user know this problem.
So it's safe to remove this.
|
Kubernetes integration test starting |
|
Kubernetes integration test status success |
|
Test build #131550 has finished for PR 30470 at commit
|
|
retest this please |
| <groupId>commons-logging</groupId> | ||
| <artifactId>commons-logging</artifactId> | ||
| </exclusion> | ||
| </exclusions> |
There was a problem hiding this comment.
Is it really unused in hadoop-client-runtime?
cc @sunchao
There was a problem hiding this comment.
This is a runtime dependency for hadoop-client-runtime, so I'm not sure if we can remove it.
There was a problem hiding this comment.
This is a runtime dependency for
hadoop-client-runtime, so I'm not sure if we can remove it.
I can test tis since we also have a env of hadoop-3.2.1 and hadoop-3.3.0. I will test this later
There was a problem hiding this comment.
Is it really unused in
hadoop-client-runtime?cc @sunchao
Seems ok.
There was a problem hiding this comment.
This is a runtime dependency for
hadoop-client-runtime, so I'm not sure if we can remove it.
Test use buildin and maven of hadoop-3.2 and hive-2.3.7, create dir and write data is ok.
There was a problem hiding this comment.
I'm always worried at some point something adds the slf4j -> commons-logging binding and then we have something circular here :) this might be fine. Is it helpful, like does it help get more log messages to the same place?
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Test build #131557 has finished for PR 30470 at commit
|
|
+CC @xkrogen |
|
Test build #131583 has finished for PR 30470 at commit
|
|
retest this please |
|
Test build #131621 has finished for PR 30470 at commit
|
|
retest this please |
|
Test build #131648 has finished for PR 30470 at commit
|
|
Good to see this cleanup! @AngersZhuuuu I guess you have checked for any remaining outstanding |
|
retest this please |
|
Test build #131703 has finished for PR 30470 at commit
|
|
retest this please |
Yea, |
|
Hope someone have time to reconfirm #30470 (comment) and #30470 (comment). Since UT can't cover real product env. |
|
Test build #131715 has finished for PR 30470 at commit
|
|
retest this please |
|
Test build #131737 has finished for PR 30470 at commit
|
|
BTW, we are reinvestigating the Hadoop dependencies now due to HADOOP-16080 . Shall we hold on this PR until we make a decision? The decision will be made tomorrow (Monday). In the worst case, we need to revert it via #30508 |
Yea, wait for your discussion and hope know your decision and detail reason. Also I am worried about #30508 and #29843 since recently we are deploying hadoop-3.3.0 since we want use Hadoop |
|
ping @dongjoon-hyun What should I do for this issue next? |
|
Sorry for the delay, @AngersZhuuuu . We reverted the hadoop-runtime-client as I wrote (#30470 (comment)). But, we are still working on it to merge back with the latest Hadoop 3.2.2. Please hold on a little bit more. Thank you for your patience. cc @sunchao |
|
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable. |
|
@dongjoon-hyun Since hadoop 3.3 supported, so can reopen this? |
What changes were proposed in this pull request?
Remove dependency of
commons-loggingWhy are the changes needed?
resolve jcl-over-slf4j conflicts with commons-logging.jar
Does this PR introduce any user-facing change?
No
How was this patch tested?
Not need