-
Notifications
You must be signed in to change notification settings - Fork 28k
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
[SPARK-48214][INFRA] Ban import org.slf4j.Logger
& org.slf4j.LoggerFactory
#46502
Conversation
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.
It seems to reveal that we have too many exceptions for this new rule though.
B.And only one place in this module use 'slf4j', as shown below: spark/common/kvstore/src/main/java/org/apache/spark/util/kvstore/LevelDBIterator.java Line 324 in e1fb1d7
And we found that this error level log does not use variables So at present, it seems that migration is not necessary . Of course, migration is also possible.
|
I am +1 for the idea. |
Well, it makes sense, |
Or how about having these modules depend on the |
sh dev/lint-java
|
Done. |
Ready for it, @gengliangwang @dongjoon-hyun @LuciferYang |
@panbingkun let's hold on this until we have a conclusion in #46600 |
@gengliangwang |
public class SparkLoggerFactory { | ||
|
||
public static SparkLogger getLogger(String name) { | ||
org.slf4j.Logger slf4jLogger = org.slf4j.LoggerFactory.getLogger(name); | ||
return new SparkLogger(slf4jLogger); | ||
return new SparkLogger(LoggerFactory.getLogger(name)); |
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.
yeah this looks better
Thanks, merging to master |
What changes were proposed in this pull request?
The pr aims to ban import
org.slf4j.Logger
&org.slf4j.LoggerFactory
.Why are the changes needed?
After the migration of structured logs on the
java side
is completed, we need to ban importorg.slf4j.Logger
&org.slf4j.LoggerFactory
in the code to avoid the log format that is not written as required in the future new java code.Does this PR introduce any user-facing change?
Yes, only for spark developers.
How was this patch tested?
Was this patch authored or co-authored using generative AI tooling?
No.