Skip to content

[SPARK-47538][BUILD] Remove commons-logging dependency#45687

Closed
dongjoon-hyun wants to merge 3 commits intoapache:masterfrom
dongjoon-hyun:commons-logging
Closed

[SPARK-47538][BUILD] Remove commons-logging dependency#45687
dongjoon-hyun wants to merge 3 commits intoapache:masterfrom
dongjoon-hyun:commons-logging

Conversation

@dongjoon-hyun
Copy link
Copy Markdown
Member

@dongjoon-hyun dongjoon-hyun commented Mar 25, 2024

What changes were proposed in this pull request?

This PR aims to remove commons-logging dependency in favor of jcl-over-slf4j.

Why are the changes needed?

To ease migration to SLF4J from JCL, SLF4J distributions include the jar file jcl-over-slf4j.jar. This jar file is intended as a drop-in replacement for JCL version 1.1.1. It implements the public API of JCL but using SLF4J underneath, hence the name "JCL over SLF4J."

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass the CIs.

Was this patch authored or co-authored using generative AI tooling?

No.

@github-actions github-actions bot added the BUILD label Mar 25, 2024
@pan3793
Copy link
Copy Markdown
Member

pan3793 commented Mar 25, 2024

https://slf4j.org/legacy.html#jclOverSLF4J

jcl-over-slf4j is intended as a drop-in replacement for commons-logging, maybe we should exclude commons-logging in all places.

@dongjoon-hyun
Copy link
Copy Markdown
Member Author

Thanks. Let me try that way, @pan3793 .

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-XXX][BUILD] Use transitive commons-logging dependency [SPARK-XXX][BUILD] Remove commons-logging dependency Mar 25, 2024
@dongjoon-hyun dongjoon-hyun changed the title [SPARK-XXX][BUILD] Remove commons-logging dependency [SPARK-47538][BUILD] Remove commons-logging dependency Mar 25, 2024
pom.xml Outdated
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record, since Apache Hive 2.0.0 removed direct commons-logging dependency via HIVE-12237 Use slf4j as logging facade, we don't need this.

@dongjoon-hyun dongjoon-hyun marked this pull request as ready for review March 25, 2024 03:54
@dongjoon-hyun
Copy link
Copy Markdown
Member Author

Thank you, @pan3793 .

@dongjoon-hyun
Copy link
Copy Markdown
Member Author

All tests passed. Could you review this dependency PR, @yaooqinn ?

@yaooqinn
Copy link
Copy Markdown
Member

Hi @dongjoon-hyun. We can also remove the lines from the NOTICE*/LICENSE*?

Copy link
Copy Markdown
Member

@yaooqinn yaooqinn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, one one minor comment

@dongjoon-hyun
Copy link
Copy Markdown
Member Author

Oh, thanks. Sure!

@dongjoon-hyun
Copy link
Copy Markdown
Member Author

Thank you, @pan3793 , @yaooqinn , @HyukjinKwon !
Merged to master for Apache Spark 4.0.0.

@dongjoon-hyun dongjoon-hyun deleted the commons-logging branch March 25, 2024 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants