-
Notifications
You must be signed in to change notification settings - Fork 28.6k
[SPARK-35420][BUILD] Replace the usage of toStringHelper with ToStringBuilder #32567
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
cc @srowen FYI |
Test build #138600 has finished for PR 32567 at commit
|
Kubernetes integration test unable to build dist. exiting with code: 1 |
The master branch compilation is recovered. Could you rebase to the master, @sarutak ? |
@dongjoon-hyun Thank you for letting me know! |
…old-guava-usage
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.
+1, LGTM.
Kubernetes integration test starting |
Kubernetes integration test status failure |
Test build #138604 has finished for PR 32567 at commit
|
Thanks all. Merging to |
… which is incompatible with newer versions ### What changes were proposed in this pull request? This PR adds rules to `checkstyle.xml` and `scalastyle-config.xml` to avoid introducing `Objects.toStringHelper` a Guava's API which is no longer present in newer Guava. ### Why are the changes needed? SPARK-30272 (#26911) replaced `Objects.toStringHelper` which is an APIs Guava 14 provides with `commons.lang3` API because `Objects.toStringHelper` is no longer present in newer Guava. But toStringHelper was introduced into Spark again and replaced them in SPARK-35420 (#32567). I think it's better to have a style rule to avoid such repetition. SPARK-30272 replaced some APIs aside from `Objects.toStringHelper` but `Objects.toStringHelper` seems to affect Spark for now so I add rules only for it. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? I confirmed that `lint-java` and `lint-scala` detect the usage of `toStringHelper` and let the lint check fail. ``` $ dev/lint-java exec: curl --silent --show-error -L https://downloads.lightbend.com/scala/2.12.14/scala-2.12.14.tgz Using `mvn` from path: /opt/maven/3.6.3//bin/mvn Checkstyle checks failed at following occurrences: [ERROR] src/main/java/org/apache/spark/network/protocol/OneWayMessage.java:[78] (regexp) RegexpSinglelineJava: Avoid using Object.toStringHelper. Use ToStringBuilder instead. $ dev/lint-scala Scalastyle checks failed at following occurrences: [error] /home/kou/work/oss/spark/core/src/main/scala/org/apache/spark/rdd/RDD.scala:93:25: Avoid using Object.toStringHelper. Use ToStringBuilder instead. [error] Total time: 25 s, completed 2021/06/02 16:18:25 ``` Closes #32740 from sarutak/style-rule-for-guava. Authored-by: Kousuke Saruta <sarutak@oss.nttdata.com> Signed-off-by: Kousuke Saruta <sarutak@oss.nttdata.com>
What changes were proposed in this pull request?
This PR replaces
toStringHelper
, an API which breaks in Guava 27.Why are the changes needed?
SPARK-30272 (#26911) removed usages which breaks in Guava 27 but
toStringHelper
is instroduced again.Does this PR introduce any user-facing change?
No.
How was this patch tested?
Build successfully finished with the following command.