You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "tomncooper (via GitHub)" <gi...@apache.org> on 2025/07/07 15:58:29 UTC
[PR] [FLINK-38048] Update java version and move parent pom to v1.2.0 [flink-connector-shared-utils]
tomncooper opened a new pull request, #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48
To bring the Flink Connectors parent pom in-line with the Flink parent pom, this PR:
* Sets the Java source version to 11
* Set the Java target version to 17
* Moves all the plugin versions into the properties section for easier admin
* Aligns the maven plugin version to flink-parent versions
* Removes Java 8 and add Java 17 to the CI test matrix
* Update GH Action steps in CI to the latest versions
My lingering question here is around the release profile, I have used source 11 and target 17 but this would prevent building with Java 11 (of course the output would still run on 11). Is that an issue?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v1.2.0 [flink-connector-shared-utils]
Posted by "echauchot (via GitHub)" <gi...@apache.org>.
echauchot commented on PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#issuecomment-3061405559
> Thanks @echauchot I will try the setup you suggested.
>
> > Thanks. The test should be done with java 11 and 17.
>
> As we are setting source to 11 and target to 17, to match Flink 2.0.0+, we cannot build projects with Java 11. We could run them on 11 but we would have to build them with 17.
>
> Building with 17 and dropping 11 seems to match what the main Flink repo is doing. So I think it make sense to stick with 17+ for this. Users who want to stick with 11 could stick with parent pom 1.1.0.
I just checked, flink parent sets both source and target to 17 we should do the same I think.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "ferenc-csaky (via GitHub)" <gi...@apache.org>.
ferenc-csaky merged PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2662343039
##########
pom.xml:
##########
@@ -60,9 +60,8 @@ under the License.
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
- <target.java.version>1.8</target.java.version>
- <maven.compiler.source>${target.java.version}</maven.compiler.source>
- <maven.compiler.target>${target.java.version}</maven.compiler.target>
+ <maven.compiler.source>17</maven.compiler.source>
+ <maven.compiler.target>17</maven.compiler.target>
Review Comment:
@tomncooper You make a good point. My worry is, in addition to the IDE, this is inconsistent with Flink maven builds. In a dev environment, `mvn clean install` for a connector and Flink - would have different targeted jdks (unless a profile is specified). This means that, without a profile I would need a different level of JAVA to compile the connectors and Flink locally.
When we get Flink changed (I assume in a future version), then we could produce another level of this parent connector and match those default levels. WDYT?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2676786889
##########
pom.xml:
##########
@@ -717,14 +738,15 @@ under the License.
</property>
</activation>
<properties>
- <target.java.version>1.8</target.java.version>
+ <maven.compiler.source>11</maven.compiler.source>
+ <maven.compiler.target>17</maven.compiler.target>
Review Comment:
@ferenc-csaky I can see that makes sense if we expect downstream repos to support different min Java versions.
I assume a connector could always release with JDK11 and JDK17 by the connector supplying an appropriate java level in a release profile
I think we are saying that the parent should not be prescriptive about the source and target java level in any way.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "ferenc-csaky (via GitHub)" <gi...@apache.org>.
ferenc-csaky commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2661512256
##########
pom.xml:
##########
@@ -60,9 +60,8 @@ under the License.
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
- <target.java.version>1.8</target.java.version>
- <maven.compiler.source>${target.java.version}</maven.compiler.source>
- <maven.compiler.target>${target.java.version}</maven.compiler.target>
+ <maven.compiler.source>17</maven.compiler.source>
+ <maven.compiler.target>17</maven.compiler.target>
Review Comment:
Exactly
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "tomncooper (via GitHub)" <gi...@apache.org>.
tomncooper commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2661213809
##########
pom.xml:
##########
@@ -60,9 +60,8 @@ under the License.
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
- <target.java.version>1.8</target.java.version>
- <maven.compiler.source>${target.java.version}</maven.compiler.source>
- <maven.compiler.target>${target.java.version}</maven.compiler.target>
+ <maven.compiler.source>17</maven.compiler.source>
+ <maven.compiler.target>17</maven.compiler.target>
Review Comment:
So both source and target should be 11 or just source?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "ferenc-csaky (via GitHub)" <gi...@apache.org>.
ferenc-csaky commented on PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#issuecomment-3729324480
Fine, lets go with 11 for source and 17 for target as the default compiler props:
```xml
<maven.compiler.source>11</maven.compiler.source>
<maven.compiler.target>17</maven.compiler.target>
```
I updated the core repo which now does this as well.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v1.2.0 [flink-connector-shared-utils]
Posted by "tomncooper (via GitHub)" <gi...@apache.org>.
tomncooper commented on PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#issuecomment-3052163089
Thanks @echauchot I will try the setup you suggested.
> Thanks. The test should be done with java 11 and 17.
As we are setting source to 11 and target to 17, to match Flink 2.0.0+, we cannot build projects with Java 11. We could run them on 11 but we would have to build them with 17.
Building with 17 and dropping 11 seems to match what the main Flink repo is doing. So I think it make sense to stick with 17+ for this. Users who want to stick with 11 could stick with parent pom 1.1.0.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#issuecomment-3715065352
or 4. Set source 11 and target to 17 to match the current core Flink.
I am pro 4 for consistency, then 2 & 3, when Flink changes. Tom is pro 2.
WDYT @ferenc-csaky ? As this choice does not have a big impact, if you are happy with option 2 - that would be fine with me.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "ferenc-csaky (via GitHub)" <gi...@apache.org>.
ferenc-csaky commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2677004020
##########
pom.xml:
##########
@@ -717,14 +738,15 @@ under the License.
</property>
</activation>
<properties>
- <target.java.version>1.8</target.java.version>
+ <maven.compiler.source>11</maven.compiler.source>
+ <maven.compiler.target>17</maven.compiler.target>
Review Comment:
Actually, I think for now we should bake Java11 into this release profile, for both source and target. And when the time comes for the first connector to drop Java11 support (if it even comes...), then we can release a new parent that will be pointed to Java17.
TBH I do not like mixing these stuff, it will just bite us in the butt cause 6 months from now nobody will remember what the heck was the intent if we overcomplicate this, so I'd like to keep it as simple as it can be.
We can either do different release profiles, but regarding this I think we should ask the community to gather more input. So for this specific PR I'd vote for locking down Java 11 for both source and target in the release profile, cause we won't need a Java17 release in the foreseeable future.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2661898219
##########
pom.xml:
##########
@@ -60,9 +60,8 @@ under the License.
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
- <target.java.version>1.8</target.java.version>
- <maven.compiler.source>${target.java.version}</maven.compiler.source>
- <maven.compiler.target>${target.java.version}</maven.compiler.target>
+ <maven.compiler.source>17</maven.compiler.source>
+ <maven.compiler.target>17</maven.compiler.target>
Review Comment:
@ferenc-csaky thanks for your update. This is my understanding now:
I assume that to support JDK 11 a Maven build option to supply `maven.compiler.target` as 11 would be needed. In order to allow this to work we need <maven.compiler.source>11</maven.compiler.source>. The `<maven.compiler.target>17</maven.compiler.target>` is just what we target by default.
So for example - this will allow the [Kafka connector CI ](https://github.com/apache/flink-connector-kafka/blob/a90e8f48a09703eeb87dd2e3de7985f678762674/.github/workflows/push_pr.yml#L32) to continue to build against jdk 11.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "tomncooper (via GitHub)" <gi...@apache.org>.
tomncooper commented on PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#issuecomment-3729345523
> @tomncooper https://github.com/apache/flink/blob/932aa4306efadc8107a33da5efa184a7c71dd14e/pom.xml#L134 shows it to be
>
> ```
> <source.java.version>11</source.java.version>
> <target.java.version>17</target.java.version>
> ```
@davidradl Yes, but the maven compiler source _was_ (before ferenc's change) set to the java target version (a few lines below):
```xml
<maven.compiler.source>${target.java.version}</maven.compiler.source>
```
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "tomncooper (via GitHub)" <gi...@apache.org>.
tomncooper commented on PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#issuecomment-3715077309
> or 4. Set source 11 and target to 17 to match the current core Flink.
The core Flink project currently sets the default compiler source and target to 17?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v1.2.0 [flink-connector-shared-utils]
Posted by "tomncooper (via GitHub)" <gi...@apache.org>.
tomncooper commented on PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#issuecomment-3049410904
Hi @echauchot, I have run the new parent pom (1.2.0) against the flink-connector-kafka project locally (using both java 17 and 21) using the same commands that the CI would run. The kafka connector is still using parent 1.0.0 so it requires updates to the checkstyle definitions, use of guava etc. But with those tweaks it builds correctly and passes all the CI tests.
I am struggling to think of a way to run the full CI ,in a repo for a connector, with the proposed 1.2.0 parent pom when that pom isn't published? Do you know of an easy way?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "ferenc-csaky (via GitHub)" <gi...@apache.org>.
ferenc-csaky commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2695187825
##########
pom.xml:
##########
@@ -717,14 +738,15 @@ under the License.
</property>
</activation>
<properties>
- <target.java.version>1.8</target.java.version>
+ <maven.compiler.source>11</maven.compiler.source>
+ <maven.compiler.target>17</maven.compiler.target>
Review Comment:
@davidradl exactly
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "tomncooper (via GitHub)" <gi...@apache.org>.
tomncooper commented on PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#issuecomment-3665703199
I have updated the parent pom to 2.0.0. I have installed the parent locally and run the Kafka connector CI locally with both Java 17 and 21 and all looks to be working correctly (after updating the `tools/maven/checkstyle.xml` to match the main Flink project's version).
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "ferenc-csaky (via GitHub)" <gi...@apache.org>.
ferenc-csaky commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2649238880
##########
pom.xml:
##########
@@ -60,9 +60,8 @@ under the License.
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
- <target.java.version>1.8</target.java.version>
- <maven.compiler.source>${target.java.version}</maven.compiler.source>
- <maven.compiler.target>${target.java.version}</maven.compiler.target>
+ <maven.compiler.source>17</maven.compiler.source>
+ <maven.compiler.target>17</maven.compiler.target>
Review Comment:
I think the default should be 11, just like it is for the core project.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v1.2.0 [flink-connector-shared-utils]
Posted by "echauchot (via GitHub)" <gi...@apache.org>.
echauchot commented on PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#issuecomment-3051924095
> Hi @echauchot, I have run the new parent pom (1.2.0) against the flink-connector-kafka project locally (using both java 17 and 21) using the same commands that the CI would run. The kafka connector is still using parent 1.0.0 so it requires updates to the checkstyle definitions, use of guava etc. But with those tweaks it builds correctly and passes all the CI tests.
>
> I am struggling to think of a way to run the full CI ,in a repo for a connector, with the proposed 1.2.0 parent pom when that pom isn't published? Do you know of an easy way?
Thanks. The test should be done with java 11 and 17.
When I did changes on the parent pom, I tested this way:
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "ferenc-csaky (via GitHub)" <gi...@apache.org>.
ferenc-csaky commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2676671775
##########
pom.xml:
##########
@@ -717,14 +738,15 @@ under the License.
</property>
</activation>
<properties>
- <target.java.version>1.8</target.java.version>
+ <maven.compiler.source>11</maven.compiler.source>
+ <maven.compiler.target>17</maven.compiler.target>
Review Comment:
I'm pretty sure we cannot do this here.
Currently, I think every connector still supports JDK11, so if we write something here as a target version it has to be JDK11, cause the release artifacts must be built for the oldest supported version (core Flink has 11 as target version in the `release` profile).
But as a connector parent, downstream repos might end up supporting different min Java versions, so I think what's feasible here is to not overwrite any java version property and just fetch `maven.compiler.target` inside `requireJavaVersion` as it is now.
That way, one will be able to release with JDK11 and JDK17 adding the given target profile.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "ferenc-csaky (via GitHub)" <gi...@apache.org>.
ferenc-csaky commented on PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#issuecomment-3729360982
@tomncooper i just changed that
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#issuecomment-3715280700
@tomncooper [https://github.com/apache/flink/blob/932aa4306efadc8107a33da5efa184a7c71dd14e/pom.xml#L134](https://github.com/apache/flink/blob/932aa4306efadc8107a33da5efa184a7c71dd14e/pom.xml#L134) shows it to be
```
<source.java.version>11</source.java.version>
<target.java.version>17</target.java.version>
```
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "ferenc-csaky (via GitHub)" <gi...@apache.org>.
ferenc-csaky commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2661776679
##########
pom.xml:
##########
@@ -60,9 +60,8 @@ under the License.
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
- <target.java.version>1.8</target.java.version>
- <maven.compiler.source>${target.java.version}</maven.compiler.source>
- <maven.compiler.target>${target.java.version}</maven.compiler.target>
+ <maven.compiler.source>17</maven.compiler.source>
+ <maven.compiler.target>17</maven.compiler.target>
Review Comment:
Since Flink 2.x still supports JDK11 IMO it is wrong to differ from that here. If a connector drops JDK11 support specifically for whatever version it's still fine, anybody who develops those features locally can use the jdk17-target profile.
Mixing source/target versions also do not really makes too much sense in our case (and I mean this for both the core Flink repo and this), as we have separate profiles, and CI also runs with specific JDK versions.
So the only case these props actually matter is to a dev who loads up Flink or connector code in their IDE, and what that recognizes by default. And if that recognizes JDK17, and then only the build fails because it will acutally run for JDK11, it's worse than opting to a newer JDK profile. At least that's my take on this.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "tomncooper (via GitHub)" <gi...@apache.org>.
tomncooper commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2662246647
##########
pom.xml:
##########
@@ -60,9 +60,8 @@ under the License.
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
- <target.java.version>1.8</target.java.version>
- <maven.compiler.source>${target.java.version}</maven.compiler.source>
- <maven.compiler.target>${target.java.version}</maven.compiler.target>
+ <maven.compiler.source>17</maven.compiler.source>
+ <maven.compiler.target>17</maven.compiler.target>
Review Comment:
I set the compiler source and target to match the current Flink 2.0 main project default which is JDK 17 for both?
```
<source.java.version>11</source.java.version>
<target.java.version>17</target.java.version>
...
<maven.compiler.source>${target.java.version}</maven.compiler.source>
<maven.compiler.target>${target.java.version}</maven.compiler.target>
```
I realise (from @ferenc-csaky's comment above) that this might actually be a typo/misconfiguration in the main project. I also agree that the profiles make this all somewhat of a moot point.
So should we just settle on these pom compiler defaults being **both** set to the lowest _supported_ JDK version, in this case JDK 11?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2664187999
##########
.github/workflows/ci.yml:
##########
@@ -24,19 +24,19 @@ jobs:
MVN_COMMON_OPTIONS: -U -B --no-transfer-progress
strategy:
matrix:
- java: [8, 11]
+ java: [17, 21]
Review Comment:
we should have 11 here, as per the recent conversation.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "tomncooper (via GitHub)" <gi...@apache.org>.
tomncooper commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2695235346
##########
pom.xml:
##########
@@ -717,14 +738,15 @@ under the License.
</property>
</activation>
<properties>
- <target.java.version>1.8</target.java.version>
+ <maven.compiler.source>11</maven.compiler.source>
+ <maven.compiler.target>17</maven.compiler.target>
Review Comment:
Ok I have updated the pom in line with this discussion.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2661710809
##########
pom.xml:
##########
@@ -60,9 +60,8 @@ under the License.
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
- <target.java.version>1.8</target.java.version>
- <maven.compiler.source>${target.java.version}</maven.compiler.source>
- <maven.compiler.target>${target.java.version}</maven.compiler.target>
+ <maven.compiler.source>17</maven.compiler.source>
+ <maven.compiler.target>17</maven.compiler.target>
Review Comment:
@tomncooper I think it should be the same as the [Flink 2.2 pom](https://github.com/apache/flink/blob/932aa4306efadc8107a33da5efa184a7c71dd14e/pom.xml#L134) source should be 11, target 17
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v1.2.0 [flink-connector-shared-utils]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#issuecomment-3659878521
@ferenc-csaky could you have a look at this one, I raised one small issue, but it would be good to get an up to date parent connector that lines up with Flink 2.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "ferenc-csaky (via GitHub)" <gi...@apache.org>.
ferenc-csaky commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2649238880
##########
pom.xml:
##########
@@ -60,9 +60,8 @@ under the License.
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
- <target.java.version>1.8</target.java.version>
- <maven.compiler.source>${target.java.version}</maven.compiler.source>
- <maven.compiler.target>${target.java.version}</maven.compiler.target>
+ <maven.compiler.source>17</maven.compiler.source>
+ <maven.compiler.target>17</maven.compiler.target>
Review Comment:
I think the default should be 11, just like it is for the core project.
Edit: I see the prev conversation, and `maven.compiler.*` is set to 17 in the core project, yes, but I believe that is a mistake, which we should not follow here (I'll follow this up in the core project). Java 11 is still supported, so the code should be limited to that both here and there, even locally. And I'd keep the JDK11 CI run too.
Sorry for the back and forth. :P
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2661710809
##########
pom.xml:
##########
@@ -60,9 +60,8 @@ under the License.
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
- <target.java.version>1.8</target.java.version>
- <maven.compiler.source>${target.java.version}</maven.compiler.source>
- <maven.compiler.target>${target.java.version}</maven.compiler.target>
+ <maven.compiler.source>17</maven.compiler.source>
+ <maven.compiler.target>17</maven.compiler.target>
Review Comment:
@tomncooper I think it should be the same as the [Flink 2.2 pom](https://github.com/apache/flink/blob/932aa4306efadc8107a33da5efa184a7c71dd14e/pom.xml#L134) source should be 11, target 17. I assume the thinking is that we want to support Java 17, but there may be java 11 code in the Flink code base. I do not know how the connectors might be broken if we set java 17 as the source; but I suspect to be safe we should just match the flink pom.
@ferenc-csaky any insight as to why we must have source as 11 for the connectors, if we can leave the source as 17 that would be beneficial, so people do not introduce new java 11 specific source code in new connectors.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2661898219
##########
pom.xml:
##########
@@ -60,9 +60,8 @@ under the License.
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
- <target.java.version>1.8</target.java.version>
- <maven.compiler.source>${target.java.version}</maven.compiler.source>
- <maven.compiler.target>${target.java.version}</maven.compiler.target>
+ <maven.compiler.source>17</maven.compiler.source>
+ <maven.compiler.target>17</maven.compiler.target>
Review Comment:
@ferenc-csaky thanks for your update. This is my understanding now:
I assume that to support JDK 11 a Maven option to supply `maven.compiler.target` as 11. In order to allow this to work we need <maven.compiler.source>11</maven.compiler.source>. The `<maven.compiler.target>17</maven.compiler.target>` is just what we target by default.
So for example - this will allow the [Kafka connector CI ](https://github.com/apache/flink-connector-kafka/blob/a90e8f48a09703eeb87dd2e3de7985f678762674/.github/workflows/push_pr.yml#L32) to continue to build against jdk 11.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2664187999
##########
.github/workflows/ci.yml:
##########
@@ -24,19 +24,19 @@ jobs:
MVN_COMMON_OPTIONS: -U -B --no-transfer-progress
strategy:
matrix:
- java: [8, 11]
+ java: [17, 21]
Review Comment:
we should include 11 here, as per the recent conversation.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v1.2.0 [flink-connector-shared-utils]
Posted by "tomncooper (via GitHub)" <gi...@apache.org>.
tomncooper commented on PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#issuecomment-3045793736
This probably supersedes #45
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "tomncooper (via GitHub)" <gi...@apache.org>.
tomncooper commented on PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#issuecomment-3714993945
So @ferenc-csaky & @davidradl what are we gonna do here?
1. Set source and target to 11 and accept the misalignment with the main Flink project.
2. Set source 17 and target 17 and match the current Flink project config, accepting that building with 11 would need to use one of the profiles. If the main Flink project changes in future we can do another release which matches that.
3. Wait for the main Flink project to clarify what those values should be and then align on them.
Personally, I would prefer option 2 as I would like to encourage the adoption of more modern SDKs but I understand that one of Flink selling points its support of older versions and JDKs.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2690819116
##########
pom.xml:
##########
@@ -717,14 +738,15 @@ under the License.
</property>
</activation>
<properties>
- <target.java.version>1.8</target.java.version>
+ <maven.compiler.source>11</maven.compiler.source>
+ <maven.compiler.target>17</maven.compiler.target>
Review Comment:
Ok so we say:
source 11 target 17 for the pom defaults
source 11 target 11 for the release profile.
I am happy with that.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v2.0.0 [flink-connector-shared-utils]
Posted by "ferenc-csaky (via GitHub)" <gi...@apache.org>.
ferenc-csaky commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2676671775
##########
pom.xml:
##########
@@ -717,14 +738,15 @@ under the License.
</property>
</activation>
<properties>
- <target.java.version>1.8</target.java.version>
+ <maven.compiler.source>11</maven.compiler.source>
+ <maven.compiler.target>17</maven.compiler.target>
Review Comment:
I'm pretty sure we cannot do this here.
Currently, I think every connector still supports JDK11, so if we write something here as a target version it is JDK11, cause the release artifacts must be built for the oldest supported version (core Flink has 11 as target version in the `release` profile).
But as a connector parent, downstream repos might end up supporting different min Java versions, so I think what's feasible here is to not overwrite any java version property and just fetch `maven.compiler.target` inside `requireJavaVersion` as it is now.
That way, one will be able to release with JDK11 and JDK17 adding the given target profile.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v1.2.0 [flink-connector-shared-utils]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2622717798
##########
pom.xml:
##########
@@ -89,14 +88,35 @@ under the License.
It MUST be a space-separated list not containing any newlines,
of entries in the form '[-]{2}add-[opens|exports]=<module>/<package>=ALL-UNNAMED'.-->
<surefire.module.config/>
+
+ <!-- Maven plugin versions -->
+ <apache-rat-plugin.version>0.13</apache-rat-plugin.version>
+ <checkstyle.version>10.18.2</checkstyle.version>
+ <cyclonedx-maven-plugin.version>2.9.1</cyclonedx-maven-plugin.version>
+ <directory-maven-plugin.version>0.1</directory-maven-plugin.version>
+ <exec-maven-plugin.version>3.5.1</exec-maven-plugin.version>
+ <google-java-format.version>1.24.0</google-java-format.version>
+ <japicmp-maven-plugin.version>0.18.5</japicmp-maven-plugin.version>
+ <maven-checkstyle-plugin.version>3.3.1</maven-checkstyle-plugin.version>
+ <maven-compiler-plugin.version>3.14.0</maven-compiler-plugin.version>
+ <maven-eclipse-plugin.version>2.8</maven-eclipse-plugin.version>
+ <maven-enforcer-plugin.version>3.1.0</maven-enforcer-plugin.version>
+ <maven-gpg-plugin.version>1.4</maven-gpg-plugin.version>
+ <maven-jar-plugin.version>2.4</maven-jar-plugin.version>
+ <maven-javadoc-plugin.version>3.8.0</maven-javadoc-plugin.version>
+ <maven-release-plugin.version>2.1</maven-release-plugin.version>
+ <maven-shade-plugin.version>3.5.3</maven-shade-plugin.version>
Review Comment:
this is now [3.6.1](https://github.com/apache/flink/blob/5954b5b579c6d4bad779d20b152eb6147b098ac6/pom.xml#L2326)
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38048] Update java version and move parent pom to v1.2.0 [flink-connector-shared-utils]
Posted by "ferenc-csaky (via GitHub)" <gi...@apache.org>.
ferenc-csaky commented on code in PR #48:
URL: https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2619762639
##########
pom.xml:
##########
@@ -27,7 +27,7 @@ under the License.
<groupId>org.apache.flink</groupId>
<artifactId>flink-connector-parent</artifactId>
- <version>1.1.0</version>
+ <version>1.2.0</version>
Review Comment:
This should be bumped to 2.0.0, cause this is a breaking change.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org