pan3793 commented on PR #51174: URL: https://github.com/apache/spark/pull/51174#issuecomment-2975209284
@dongjoon-hyun thanks for your review and explanation about your concerns. first of all, sorry if I make thing confusing, to clarify, my goal is not "remove the commons-lang3 dependency completely", actually, unlike Guava, commons-lang3 keeps pretty good backward-compatibility. and I understand that the change in versioning policy during Java 8 caused some confusion, for projects that supports Java 8 and above, I agree that common-lang3 might be the best and simplest way to handle the JDK vesion, but given now Spark requires Java 17+ and modern JDKs provide simple and stable API (via [JEP 223](https://openjdk.org/jeps/223)) to get and compare the version (in additional to feature(major) version of JDK, JEP 223 API also provides interim/update/patch version, which was used in SPARK-50946), switching to JEP 223 API seems to be a future-proofing selection. >> Spark has no guarantee to work with older commons-lang3 > > In short, it's not a good idea to downgrade Apache Spark's jar dependency with a user-provide one. And, this PR might give a wrong idea to the users which we allow (and guarantee) a user to downgrade it. In fact, we should not and never recommend that way. I absolutely agree with that, I strongly discourage our users/customers to use `spark.[driver|executor].userClassPathFirst=true`. but on the other hand, these two configs were exposed to user docs for a long time, and some users do rely it to address some class conflict issues. I would make some minor changes that has no hurt on Spark codebase to make it work better with older version of some libraries. regards to `commons-lang3`, Spark uses only a few set of its API, all are present for a long time except to those JAVA_N constants. actually, the JAVA_N constants are added frequently and have a delay with the JDK release, for example, the latest `commons-lang3:3.17.0` only has `JAVA_22`, which was added since `3.15.0`, but the latest JDK version is 24, which makes us can not write code like `SystemUtils.isJavaVersionAtLeast(JAVA_23)` In short, I think there is no obviously drawback in switching to JEP 223 API, and I can update the PR description to remove the commons-lang3 compatibility words if it cause confusing. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
