Greg, 1. Ok sounds good we can target JDK 17 in this KIP if we decide to do that. 2. For the EE version, I don't think it really matters since we won't be using any new features that I am aware of. It's just something I noticed that we will need to pick because Jetty 12 supports multiple versions so it would affect which spec jars we use. In the past Jetty versions have been tied to a specific Servlet spec but the new Jetty 12 they have abstracted things away and they support multiple versions simultaneously. There's different versions for all the specs but the primary one to note for us would be that JavaEE 9 uses the Servlet 5.0 spec and JavaEE 10 uses the Servlet 6.0 spec. JavaEE 11 is under development and will use the Servlet 6.1 spec. So we may not really need to call out the EE version at all if it doesn't matter and we are not using specific features but I wanted to bring it up since multiple versions are listed as being compatible with Jetty 12 so we need to pick one. On the main page they list the different servlet specs they support: https://eclipse.dev/jetty/ 3. Right, I didn't mean we should include it in the KIP, I was more asking I guess how to go about things. It looks like we could use a lot of it and adapt the work already done. While it's under the Apache 2.0 license and we could use it, someone else wrote it so it would still be good to properly credit that person as you mentioned. If I work on it I would probably start over with a new branch and just use the old PR as a guide and then maybe figure out a way to credit the original author. There's always that co-author tag that could be used I think.
Chris On Mon, Apr 1, 2024 at 12:11 PM Greg Harris <greg.har...@aiven.io.invalid> wrote: > Hey Chris, > > Thanks for your questions! > > 1. KIPs are generally immutable once they've been voted on. In this > case, I think it's better that this KIP include the bump to Java 17 > just for Connect and MirrorMaker 2, and should include that in the KIP > title. > 2. I'm not familiar with the difference, can you provide some more > context that would help us make a decision? AFAIU we haven't specified > an EE version in the past, and we don't do any sort of automated > testing for compatibility. I think it would be good to call out which > components have JavaEE-sensitive dependencies (just connect-runtime?). > We do not want to accidentally give users the idea that the clients > depend on the JavaEE version, as that could be very confusing. > 3. That's an implementation detail left up to anyone that effects this > KIP on the repo, and doesn't need to be mentioned in the KIP itself. I > have seen people adopt changes from un-merged PRs after the original > contributor has lost interest, while still crediting the original > contributor for their portion of the changes. If you're doing this, > then it's ultimately up to your judgement. > > Thanks, > Greg > > On Mon, Apr 1, 2024 at 6:30 AM Christopher Shannon > <christopher.l.shan...@gmail.com> wrote: > > > > Hi Greg, > > > > Thanks for the detailed analysis on the connect framework. It sounds like > > hopefully we can go ahead and require JDK 17+ and bump that dependency > for > > the ConnectRestExtensionContext. > > > > I agree we can leave it open and hear what others think as well about the > > requirement, if everyone ends up agreeing, I can update the KIP to > reflect > > requiring JDK 17 and going with Jetty 12. > > > > I had a couple of questions > > 1) If we go with JDK 17 as a requirement for the Connect framework, > should > > we modify and make that part of KIP-1013 or keep it in this one? > > 2) Should we go with JavaEE 9 or JavaEE 10? I'm not sure how much it > > matters in this case. > > 3) Can we just re-open https://github.com/apache/kafka/pull/10176 as a > > starting point or maybe we can create a new PR and use it as a basis? > It's > > a bit old so I suspect there would be a ton of conflicts so it might be > > best to start over and use it as a guide. I can work on a new PR once we > > are all on the same page. I don't think it would take too long to put > > together since most of the work is just dependency updates and package > > renaming. > > > > Chris > > > > > > On Fri, Mar 29, 2024 at 8:59 PM Greg Harris <greg.har...@aiven.io.invalid > > > > wrote: > > > > > Hey all, > > > > > > I looked into how Debezium handled the javax->jakarta changeover for > > > Quarkus, and found this release note: > > > > > > > https://debezium.io/blog/2023/04/20/debezium-2-2-final-released/#new-quarkus-3 > > > It appears that Debezium 2.1 required Quarkus < 3.0, and Debezium 2.2 > > > required Quarkus >= 3.0. The upgrade for Kafka could be very similar > > > and not incur a major version release. > > > > > > We can leave some time to hear from anyone else that is impacted by > > > this change, but from the open source projects present on github, this > > > LGTM. > > > > > > Thanks, > > > Greg > > > > > > On Fri, Mar 29, 2024 at 5:27 PM Greg Harris <greg.har...@aiven.io> > wrote: > > > > > > > > Hi Chris, > > > > > > > > Thank you so much for opening this KIP, and making sure Kafka keeps > up > > > > with the rest of the Java ecosystem! > > > > > > > > I took a look around at some Open Source connector implementations, > > > > and checked their Java version support: > > > > * The Aiven connect plugins (http, bigquery, jdbc, elasticsearch, > > > > opensearch, commons, s3, transforms, gcs), 6/9 are tested with JDK 17 > > > > in CI, 2/9 JDK 11, and 1/9 JDK 8. I'll look into improving the > testing > > > > matrix, but I don't expect substantial problems with requiring JDK > 17. > > > > * The Debezium Project lists Java 11+ compatibility: > > > > https://debezium.io/releases/ and appears to use Java 22 (ga) and 23 > > > > (ea) in their CI: > > > > > > > > https://github.com/debezium/debezium/blob/9cdaa38453c9f065c6075d31636592a5b147518f/.github/workflows/jdk-outreach-workflow.yml#L20 > > > > > > > > I think the bigger problem really is the ConnectRestExtension, since > > > > we've baked the rs-api type into the signature of > > > > ConnectRestExtensionContext. > > > > * Aiven doesn't have any ConnectRestExtensions, so this isn't a > concern > > > for us. > > > > * The Debezium Project has at least 6 ConnectRestExtension > > > > implementations: > > > > > > > > https://github.com/search?q=repo%3Adebezium%2Fdebezium+ConnectRestExtension+language%3AJava&type=code&l=Java > > > > . Some of these are baked into artifacts that I know for a fact are > > > > used in normal connect deployments. > > > > * I found a healthcheck extension that looks unmaintained: > > > > > > > > https://github.com/LoObp4ck/kafka-connect-healthchecks/blob/2d9dbfee900d9f85e6acd9a09bd04969afa46261/src/main/java/com/loobpack/data/kafka/connect/healthcheck/extension/HealthCheckConnectRestExtension.java#L9 > > > > > > > > I figure that adopting this KIP would mean that the Debezium project > > > > would be forced to bump their major version 3.0 to be compatible with > > > > Connect 4.0, or otherwise change their packaging, so I'd like to hear > > > > from the Debezium folks what they think of this proposal. > > > > > > > > Thanks, > > > > Greg > > > > > > > > On Wed, Mar 27, 2024 at 4:43 PM Christopher Shannon > > > > <christopher.l.shan...@gmail.com> wrote: > > > > > > > > > > Hi, > > > > > > > > > > I'm proposing a KIP for Kafka 4.0 to upgrade to to Jakarta and > JavaEE 9 > > > > > APIs. This will also upgrade dependencies like Jetty and move away > from > > > > > the depcrated javax namespace to be in line with other libraries > and > > > > > frameworks. There was some initial > > > > > <https://lists.apache.org/thread/p4qbqh8r77h4khn3yoof2b0gbq3wbc5q> > > > > > discussion and below is the KIP. > > > > > > > > > > Please take a look and let me know what you think: > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1032%3A+Upgrade+to+Jakarta+and+JavaEE+9+in+Kafka+4.0 > > > > > > > > > > Thanks, > > > > > Chris > > > >
