[FLINK-30274] Bump commons-collections 3.x to commons-collections4 #21442

chucheng92 · 2022-12-02T08:54:24Z

What is the purpose of the change

Apache commons-collections 3.x is a Java 1.3 compatible version, and it does not use Java 5 generics. Apache commons-collections4 4.4 is an upgraded version of commons-collections and it built by Java 8. So we bump commons-collections.

Brief change log

update pom

Verifying this change

This change is a trivial rework / code cleanup. Verifying by current total cases.

Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): yes
The public API, i.e., is any changed class annotated with @Public(Evolving): no
The serializers: no
The runtime per-record code paths (performance sensitive): no
Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
The S3 file system connector: no

Documentation

Does this pull request introduce a new feature? no
If yes, how is the feature documented? not applicable

flinkbot · 2022-12-02T09:01:41Z

CI report:

f7c796e Azure: FAILURE

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run azure re-run the last Azure build

MartijnVisser

This is not a replacement, but just adding collections4 without using it. Where's the value in that?

I would expect that a replacement also shows the refactoring of the current usage of commons-collections to commons-collections4.

chucheng92 · 2022-12-02T09:26:47Z

This is not a replacement, but just adding collections4 without using it. Where's the value in that?

I would expect that a replacement also shows the refactoring of the current usage of commons-collections to commons-collections4.

Add it in dependencyManagement to let the submodules match with this version in later using.

MartijnVisser · 2022-12-02T09:39:43Z

Again, why would we add a dependency without using it? There's no value in that. Please change current usage of commons-collections to commons-collections4. Else there's no value in the PR and I'll have to close it.

chucheng92 · 2022-12-02T10:01:48Z

Again, why would we add a dependency without using it? There's no value in that. Please change current usage of commons-collections to commons-collections4. Else there's no value in the PR and I'll have to close it.

Yes. the best solution is replacing it. U can close this pr, but pls keep issue open, i will try to replace it truely.

chucheng92 · 2023-02-08T07:10:14Z

@MartijnVisser PTAL, thanks.

MartijnVisser · 2023-02-08T08:06:44Z

@MartijnVisser PTAL, thanks.

@chucheng92 The PR can't pass the CI because it doesn't contain the necessary changes to the NOTICE files (where's there's still an outdated reference to the previous versions of commons-collections and none to the new one).

chucheng92 · 2023-02-08T08:18:39Z

@MartijnVisser thanks. Is this caused by updating the commons-collections4? It add some transitive dependencies?

INFO org.apache.flink.tools.ci.licensecheck.NoticeFileChecker [] - Problems were detected for a NOTICE file.
flink-sql-connector-hbase-2.2:
These issues are assumed to be false-positives:
Dependency com.fasterxml.jackson.core:jackson-core:2.4.0 is not bundled, but listed.
Dependency com.google.protobuf:protobuf-java:3.7.1 is not bundled, but listed.
Dependency com.google.code.gson:gson:2.8.5 is not bundled, but listed.
Dependency com.fasterxml.jackson.core:jackson-databind:2.4.0 is not bundled, but listed.
Dependency com.fasterxml.jackson.core:jackson-annotations:2.4.0 is not bundled, but listed.
Dependency org.apache.commons:commons-collections4:4.3 is not bundled, but listed.
Dependency com.google.protobuf:protobuf-java-util:3.7.1 is not bundled, but listed.
Dependency commons-logging:commons-logging:1.1.1 is not bundled, but listed.
Dependency com.google.guava:guava:27.1-jre is not bundled, but listed.
Dependency com.google.guava:failureaccess:1.0.1 is not bundled, but listed.
Dependency io.netty:netty-all:4.1.34.Final is not bundled, but listed.
Dependency commons-cli:commons-cli:1.4 is not bundled, but listed.

Can we add a NOTICE file in flink-core to address this or temporarily we do not do this updating.

MartijnVisser · 2023-02-08T08:21:29Z

Is this caused by updating the commons-collections4?

Yes

It add some transitive dependencies?

No. Like those lines say These issues are assumed to be false-positives.

The issue lies here https://dev.azure.com/apache-flink/apache-flink/_build/results?buildId=45877&view=logs&j=b59e5554-36c7-5512-ab1a-b80b74075fce&t=849d419c-1b8f-52b7-e455-d4bc36ec43ad&l=30273

	flink-dist:
		 These issue are legally problematic and MUST be fixed: 
			Dependency org.apache.commons:commons-collections4:4.4 is not listed.
		 These issues are mistakes that aren't legally problematic. They SHOULD be fixed at some point, but we don't have to: 
			Dependency commons-collections:commons-collections:3.2.2 is not bundled, but listed.

chucheng92 · 2023-02-08T08:41:58Z

Is this caused by updating the commons-collections4?

Yes

It add some transitive dependencies?

No. Like those lines say These issues are assumed to be false-positives.

The issue lies here https://dev.azure.com/apache-flink/apache-flink/_build/results?buildId=45877&view=logs&j=b59e5554-36c7-5512-ab1a-b80b74075fce&t=849d419c-1b8f-52b7-e455-d4bc36ec43ad&l=30273
	flink-dist:
		 These issue are legally problematic and MUST be fixed: 
			Dependency org.apache.commons:commons-collections4:4.4 is not listed.
		 These issues are mistakes that aren't legally problematic. They SHOULD be fixed at some point, but we don't have to: 
			Dependency commons-collections:commons-collections:3.2.2 is not bundled, but listed.

i think i already know how to do it. PR-21048 There is similar work done here. thanks.

chucheng92 · 2023-02-09T14:40:55Z

@flinkbot run azure

MartijnVisser · 2023-02-09T15:01:14Z

@chucheng92 You need to rebase your PR on the latest master

chucheng92 · 2023-02-10T07:54:24Z

@MartijnVisser thanks. i rebased it. ci passed. PTAL

MartijnVisser

@chucheng92 I still see some occurrences of org.apache.commons.collections, can we get completely get rid of everything that currently uses collections 3.x?

chucheng92 · 2023-02-14T05:11:02Z

@MartijnVisser I think we can fix it completely. i will fix this and update pr ASAP. Sorry for my mistake. thanks.

chucheng92 · 2023-02-15T06:18:51Z

chucheng92 · 2023-03-15T05:26:39Z

hi. @MartijnVisser i have updated the pr. And i searched the source code didn't find any other occurs using org.apache.commons.collections. PTAL. thanks.

hi. @MartijnVisser please take a look ?

MartijnVisser

@chucheng92 I think in general this is looking good, but I would like to wait until https://issues.apache.org/jira/browse/FLINK-30859 is completed, to avoid another round of syncing towards the Flink Kafka connector repo.

chucheng92 · 2023-03-24T02:25:57Z

@MartijnVisser thanks for explanations. make sense. we will be watching closely about kafka externalization.

chucheng92 · 2023-06-29T03:10:13Z

@MartijnVisser hi, Martijn, https://issues.apache.org/jira/browse/FLINK-30859 has been resolved. Can you help me to check this pr?

MartijnVisser

@chucheng92 There are still multiple occurrences of org.apache.commons.collections in the codebase; can you double check please?

chucheng92 · 2023-06-29T12:52:26Z

Hi @MartijnVisser thanks. it seems like some recent code introduced a old usage. I updated it.

please check.

MartijnVisser · 2023-06-30T12:24:31Z

please check.

The CI is failing for your PR. Please check

chucheng92 · 2023-06-30T15:02:46Z

@MartijnVisser There is a circular reference.

ci failed becase flink-python kafka test case failed.

flink-python test case reference flink-sql-connector-kafka（not uber jar, without commons-collections class）, so search it in flink and cause ClassNotFoundException. So we need update flink-sql-connector-kafka first.

The pr is: apache/flink-connector-kafka#38, when it's fixed this pr can merge, and finally we may remove explicit dependency in flink-connector-kafka repo.

AlvaroStream · 2024-05-16T12:30:47Z

@MartijnVisser or @chucheng92 do you think this will enter in future releases?

Thanks.

github-actions · 2025-01-15T12:08:55Z

This PR is being marked as stale since it has not had any activity in the last 180 days.
If you would like to keep this PR alive, please leave a comment asking for a review.
If the PR has merge conflicts, update it with the latest from the base branch.

If you are having difficulty finding a reviewer, please reach out to the [community](https://flink.apache.org/what-is-flink/community/).

If this PR is no longer valid or desired, please feel free to close it. If no activity occurs in the next 90 days, it will be automatically closed.

github-actions · 2025-04-02T06:25:50Z

This PR has been closed since it has not had any activity in 120 days.
If you feel like this was a mistake, or you would like to continue working on it,
please feel free to re-open the PR and ask for a review.

MartijnVisser requested changes Dec 2, 2022

View reviewed changes

chucheng92 force-pushed the FLINK-30274 branch from 9d2b892 to 236b302 Compare December 2, 2022 09:23

chucheng92 force-pushed the FLINK-30274 branch from 236b302 to 2f1c02f Compare December 2, 2022 09:31

flinkbot added the component=BuildSystem label Dec 2, 2022

chucheng92 force-pushed the FLINK-30274 branch from 2f1c02f to 0257664 Compare February 8, 2023 06:54

chucheng92 changed the title ~~[FLINK-30274][pom] Add commons-collections4 to replace commons-collec…~~ [FLINK-30274] Upgrade commons-collections 3.x to commons-collections4 Feb 8, 2023

chucheng92 force-pushed the FLINK-30274 branch from 0257664 to 4d5e314 Compare February 8, 2023 07:02

chucheng92 force-pushed the FLINK-30274 branch 3 times, most recently from 1369024 to e7c038e Compare February 9, 2023 10:13

chucheng92 force-pushed the FLINK-30274 branch from e7c038e to ffab99d Compare February 10, 2023 02:19

MartijnVisser requested changes Feb 13, 2023

View reviewed changes

chucheng92 force-pushed the FLINK-30274 branch from ffab99d to a250ea3 Compare February 15, 2023 04:21

chucheng92 changed the title ~~[FLINK-30274] Upgrade commons-collections 3.x to commons-collections4~~ [FLINK-30274] Bump commons-collections 3.x to commons-collections4 Mar 15, 2023

chucheng92 requested a review from MartijnVisser March 23, 2023 06:41

MartijnVisser reviewed Mar 23, 2023

View reviewed changes

chucheng92 force-pushed the FLINK-30274 branch 2 times, most recently from c844114 to 54bfe88 Compare June 29, 2023 03:02

chucheng92 requested a review from MartijnVisser June 29, 2023 03:11

MartijnVisser self-assigned this Jun 29, 2023

MartijnVisser requested changes Jun 29, 2023

View reviewed changes

chucheng92 requested a review from MartijnVisser June 30, 2023 01:15

chucheng92 mentioned this pull request Jun 30, 2023

[WIP][FLINK-32522] Add commons-collections in kafka-sql-connector shade jar apache/flink-connector-kafka#37

Closed

chucheng92 force-pushed the FLINK-30274 branch from 2cbdb61 to 71e44bf Compare July 3, 2023 11:55

[FLINK-30274] Bump commons-collections 3.x to commons-collections4

f7c796e

chucheng92 force-pushed the FLINK-30274 branch from 71e44bf to f7c796e Compare July 3, 2023 12:03

chucheng92 mentioned this pull request Oct 11, 2023

[FLINK-32522][connectors/kafka] Kafka connector should depend on commons-collections instead of inheriting from flink apache/flink-connector-kafka#38

Open

github-actions bot added the stale label Jan 15, 2025

github-actions bot added the closed-stale label Apr 2, 2025

github-actions bot closed this Apr 2, 2025

[FLINK-30274] Bump commons-collections 3.x to commons-collections4 #21442

[FLINK-30274] Bump commons-collections 3.x to commons-collections4 #21442

Uh oh!

Conversation

chucheng92 commented Dec 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the purpose of the change

Brief change log

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

Uh oh!

flinkbot commented Dec 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI report:

Uh oh!

MartijnVisser left a comment

Choose a reason for hiding this comment

Uh oh!

chucheng92 commented Dec 2, 2022

Uh oh!

MartijnVisser commented Dec 2, 2022

Uh oh!

chucheng92 commented Dec 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chucheng92 commented Feb 8, 2023

Uh oh!

MartijnVisser commented Feb 8, 2023

Uh oh!

chucheng92 commented Feb 8, 2023

Uh oh!

MartijnVisser commented Feb 8, 2023

Uh oh!

chucheng92 commented Feb 8, 2023

Uh oh!

chucheng92 commented Feb 9, 2023

Uh oh!

MartijnVisser commented Feb 9, 2023

Uh oh!

chucheng92 commented Feb 10, 2023

Uh oh!

MartijnVisser left a comment

Choose a reason for hiding this comment

Uh oh!

chucheng92 commented Feb 14, 2023

Uh oh!

chucheng92 commented Feb 15, 2023

Uh oh!

chucheng92 commented Mar 15, 2023

Uh oh!

MartijnVisser left a comment

Choose a reason for hiding this comment

Uh oh!

chucheng92 commented Mar 24, 2023

Uh oh!

chucheng92 commented Jun 29, 2023

Uh oh!

MartijnVisser left a comment

Choose a reason for hiding this comment

Uh oh!

chucheng92 commented Jun 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MartijnVisser commented Jun 30, 2023

Uh oh!

chucheng92 commented Jun 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlvaroStream commented May 16, 2024

Uh oh!

github-actions bot commented Jan 15, 2025

Uh oh!

github-actions bot commented Apr 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

chucheng92 commented Dec 2, 2022 •

edited

Loading

flinkbot commented Dec 2, 2022 •

edited

Loading

chucheng92 commented Dec 2, 2022 •

edited

Loading

chucheng92 commented Jun 29, 2023 •

edited

Loading

chucheng92 commented Jun 30, 2023 •

edited

Loading