The KIP has been updated to include the changes to the ConnectRestExtensionContext.
On Tue, Jul 9, 2024 at 7:16 PM Christopher Shannon < christopher.l.shan...@gmail.com> wrote: > Thanks, I missed that, I can update the KIP tomorrow to include that. > > Chris > > On Tue, Jul 9, 2024 at 6:15 PM Chris Egerton <chr...@aiven.io.invalid> > wrote: > >> Hi Chris, >> >> Thanks for the updates and the additional context. I have one last >> nit after doing another readthrough: in the public interfaces section it >> states that "There shouldn't be changes to the main client API as the API >> doesn't use jakarta". This is almost true, but like Greg has mentioned >> previously, there is the REST extension API: see the >> ConnectRestExtensionContext interface [1] in the connect-api module. I >> think we should call out that the signature of >> ConnectRestExtensionContext::configurable will change to return an >> instance >> of jakarta.ws.rs.core.Configurable instead of >> javax.ws.rs.core.Configurable. >> >> This doesn't block my approval of the KIP since I think that the above is >> an acceptable change to make, but IMO it should be added to give a better >> picture of the changes to anyone else who views the KIP. >> >> [1] - >> >> https://github.com/apache/kafka/blob/43676f7612b2155ecada54c61b129d996f58bae2/connect/api/src/main/java/org/apache/kafka/connect/rest/ConnectRestExtensionContext.java#L22 >> >> Cheers, >> >> Other Chris >> >> On Tue, Jul 9, 2024 at 3:40 PM Christopher Shannon < >> christopher.l.shan...@gmail.com> wrote: >> >> > Hi Chris, >> > >> > I can make those changes. For number 2 that was a holdover back when we >> > were discussing 11 vs 12 and whether or not JDK 17 would be used, but we >> > are going with 12 since 11 is deprecated so I will fix that. >> > >> > >> > On Tue, Jul 9, 2024 at 2:54 PM Chris Egerton <chr...@aiven.io.invalid> >> > wrote: >> > >> > > Hi Chris, >> > > >> > > Thanks for the KIP! It's a bit of a drastic change to rip the bandaid >> off >> > > like this and require users running Connect to upgrade to JDK 17, but >> I >> > > think it's the best out of the options that are available to us. >> > > >> > > Two small nits on the KIP: >> > > 1. It'd be nice to link to KIP-1013 in the motivation section to >> > establish >> > > that there is already some precedent for bumping to JDK 17+ for >> > server-side >> > > components (i.e., Kafka brokers) in 4.0. >> > > 2. In the compatibility section it's stated that "JDK 17+ will be >> > required >> > > if Jetty 12.x is chosen to be used". This is a pretty big "if". My >> > > understanding is that we will definitely be using Jetty 12.x; can we >> > remove >> > > the "if" or is it still up for debate whether we'll do that switch for >> > > Connect? >> > > >> > > Cheers, >> > > >> > > Other Chris >> > > >> > > On Tue, Jul 9, 2024 at 10:49 AM Christopher Shannon < >> > > christopher.l.shan...@gmail.com> wrote: >> > > >> > > > Hi Greg, >> > > > >> > > > I will make a couple quick tweaks and open a vote. I think we should >> > > target >> > > > JDK 17, JavaEE 10 and Jetty 12 because Jetty 11 is now EOL. >> > > > >> > > > Chris >> > > > >> > > > On Mon, Jul 8, 2024 at 2:13 PM Greg Harris >> > <greg.har...@aiven.io.invalid >> > > > >> > > > wrote: >> > > > >> > > > > Hi Chris, >> > > > > >> > > > > Please open a vote thread for this. >> > > > > >> > > > > Thanks, >> > > > > Greg >> > > > > >> > > > > On Tue, May 14, 2024 at 9:07 AM Christopher Shannon < >> > > > > christopher.l.shan...@gmail.com> wrote: >> > > > > >> > > > > > I just wanted to bump this and see if anyone had more feedback >> > before >> > > > > > trying to call a vote for this for 4.0? >> > > > > > >> > > > > > Chris >> > > > > > >> > > > > > On Mon, Apr 1, 2024 at 3:41 PM Christopher Shannon < >> > > > > > christopher.l.shan...@gmail.com> wrote: >> > > > > > >> > > > > > > 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 >> > > > > > >> > > >> > > > > > >> >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > >> >
