[#1699] improvement: Upgrade from commons-collections:commons-collections:3.2.2 to org.apache.commons:commons-collections:4.4 #1700
Conversation
rickyma
changed the title
…ollections:3.2.2 to org.apache.commons:commons-collections4.5.0-M1
|
Is this common update for other opensource projects? |
|
I think it is. You can refer to https://issues.apache.org/jira/browse/SPARK-37968. |
|
@LuciferYang Could you help me review this pull request? |
pom.xml
Outdated
| <awaitility.version>4.2.0</awaitility.version> | ||
| <checkstyle.version>9.3</checkstyle.version> | ||
| <commons-collections.version>3.2.2</commons-collections.version> | ||
| <commons-collections4.version>4.5.0-M1</commons-collections4.version> |
There was a problem hiding this comment.
I suggest migrating to 4.4 first, rather than a milestone version
rickyma
changed the title
| <groupId>commons-collections</groupId> | ||
| <artifactId>commons-collections</artifactId> | ||
| <version>${commons-collections.version}</version> | ||
| <groupId>org.apache.commons</groupId> |
There was a problem hiding this comment.
should only modify the commons-collections part, no need to touch the commons-lang3 part
There was a problem hiding this comment.
Others is fine to me. But please check whether there are cascading dependencies on commons-collections 3. If there are, whether it is necessary to retain the definition of commons-collections 3 version in uniffle. @rickyma
There was a problem hiding this comment.
I just changed the order of the dependencies. commons-lang3 remains the same as before.
The UTs have all passed, so the casading dependencies should be OK.
There was a problem hiding this comment.
The UTs have all passed, so the casading dependencies should be OK.
OK. It would be even better if we could double check this through the dependency tree.
I just changed the order of the dependencies. commons-lang3 remains the same as before.
Can we maintain the original order?
There was a problem hiding this comment.
We can find commons-collections 3 in the dependency tree which is introduced by hadoop-common, but it is provided:
[INFO] +- org.apache.hadoop:hadoop-common:jar:2.8.5:provided
[INFO] | +- commons-collections:commons-collections:jar:3.2.2:provided
|
Merged into master. Thanks @rickyma |
…in shaded clients (#1742) ### What changes were proposed in this pull request? Add `commons-collections4` dependencies in shaded clients. ### Why are the changes needed? After #1700, in some old versions of Spark, the `commons-collections4` dependency could be missing. Add the potential missing dependency to improve robustness. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Existing UTs. Tested in our env.
…in shaded clients (#1742) ### What changes were proposed in this pull request? Add `commons-collections4` dependencies in shaded clients. ### Why are the changes needed? After #1700, in some old versions of Spark, the `commons-collections4` dependency could be missing. Add the potential missing dependency to improve robustness. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Existing UTs. Tested in our env.
4 participants
What changes were proposed in this pull request?
Upgrade from commons-collections:commons-collections:3.2.2 to org.apache.commons:commons-collections:4.4.
Why are the changes needed?
Fix: #1699.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Existing UTs.