Hey Snehashis, I'm glad to hear you're still interested in this KIP! I'm happy to let you drive this, and I apologize for increasing the scope of work so drastically. To make up for that, I'll volunteer to be the primary PR reviewer to help get this done quickly once the KIP is approved.
Thanks, Greg On Fri, May 10, 2024 at 3:51 AM Snehashis <snehashisp1...@gmail.com> wrote: > > Hi Greg, > > Thanks for the follow up to my original KIP, I am in favour of the > additions made to expand its scope, the addition of range versions > specifically make a lot of sense. > > Apologies if I have not publicly worked on this KIP for a long time. The > original work was done when the move to service loading was in discussion > and I wanted to loop back to this only after that work was completed. Post > its conclusion, I have not been able to take this up due to other > priorities. If it's okay with you, I would still like to get this > implemented myself, including the additional scope. > > Thanks and regards > Snehashis > > On Fri, May 10, 2024 at 12:45 AM Greg Harris <greg.har...@aiven.io.invalid> > wrote: > > > Hi all, > > > > I'd like to reboot the discussion on KIP-891: > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-891%3A+Running+multiple+versions+of+Connector+plugins > > > > I've made some changes, most notably: > > > > 1. Specifying versions for all plugins in Connector configs > > (converters, header converters, transforms, and predicates) not just > > connectors & tasks > > 2. Specifying a range of versions instead of an exact match > > 3. New metrics to observe what versions are in-use > > > > Thanks to Snehashis for the original KIP idea! > > > > Thanks, > > Greg > > > > On Tue, Jan 2, 2024 at 11:49 AM Greg Harris <greg.har...@aiven.io> wrote: > > > > > > Hi Snehashis, > > > > > > Thank you for the KIP! This is something I've wanted for a long time. > > > > > > I know the discussion has gone cold, are you still interested in > > > pursuing this feature? I'll make time to review the KIP if you are > > > still accepting comments. > > > > > > Thanks, > > > Greg > > > > > > On Tue, Nov 22, 2022 at 12:29 PM Snehashis <snehashisp1...@gmail.com> > > wrote: > > > > > > > > Thanks for the points Sagar. > > > > > > > > > 1) Should we update the GET /connectors endpoint to include the > > version of > > > > > the plugin that is running? It could be useful to figure out the > > version > > > > of > > > > > the plugin or I am assuming it gets returned by the expand=info call? > > > > > > > > I think this is good to have and possible future enhancement. The > > version > > > > info will be present in the config of the connector if the user has > > > > specified the version. Otherwise it is the latest version which the > > user > > > > can find out from the connector-plugin endpoint. The information can be > > > > introduced to the response of the GET /connectors endpoint itself, > > however > > > > the most ideal way of doing this would be to get the currently running > > > > instance of the connector and get the version directly from there. > > This is > > > > slightly tricky as the connector could be running in a different node. > > > > One way to do this would be to persist the version information in the > > > > status backing store during instantiation of the connector. It requires > > > > some more thought and since the version is part of the configs if > > provided > > > > and evident otherwise, I have not included it in this KIP. > > > > > > > > > 2) I am not aware of this and hence asking, can 2 connectors with > > > > different > > > > > versions have the same name? Does the plugin isolation allow this? > > This > > > > > could have a bearing when using the lifecycle endpoints for > > connectors > > > > like > > > > > DELETE etc. > > > > > > > > All connectors in a cluster need to have uniquire connector names > > > > regardless of what version of the plugin the connector is running > > > > underneath. This is something enforced by the connect runtime itself. > > All > > > > connect CRUD operations are keyed on the connector name so there will > > not > > > > be an issue. > > > > > > > > Regards > > > > Snehashis > > > > > > > > On Tue, Nov 22, 2022 at 3:16 PM Sagar <sagarmeansoc...@gmail.com> > > wrote: > > > > > > > > > Hey Snehashsih, > > > > > > > > > > Thanks for the KIP. It looks like a very useful feature. Couple of > > > > > small-ish points, let me know what you think: > > > > > > > > > > 1) Should we update the GET /connectors endpoint to include the > > version of > > > > > the plugin that is running? It could be useful to figure out the > > version of > > > > > the plugin or I am assuming it gets returned by the expand=info call? > > > > > 2) I am not aware of this and hence asking, can 2 connectors with > > different > > > > > versions have the same name? Does the plugin isolation allow this? > > This > > > > > could have a bearing when using the lifecycle endpoints for > > connectors like > > > > > DELETE etc. > > > > > > > > > > Thanks! > > > > > Sagar. > > > > > > > > > > > > > > > On Tue, Nov 22, 2022 at 2:10 PM Ashwin <apan...@confluent.io.invalid > > > > > > > > wrote: > > > > > > > > > > > Hi Snehasis, > > > > > > > > > > > > > IIUC (please correct me if I am wrong here), what you highlighted > > > > > above, > > > > > > is > > > > > > a versioning scheme for a connector config for the same connector > > (and > > > > > not > > > > > > different versions of a connector plugin). > > > > > > > > > > > > Sorry for not being more precise in my wording - I meant > > registering > > > > > > versions of schema for connector config. > > > > > > > > > > > > Let's take the example of a fictional connector which uses a > > fictional > > > > > AWS > > > > > > service. > > > > > > > > > > > > Fictional Connector Config schema version:2.0 > > > > > > --- > > > > > > { > > > > > > "$schema": "http://json-schema.org/draft-04/schema#";, > > > > > > "type": "object", > > > > > > "properties": { > > > > > > "name": { > > > > > > "type": "string" > > > > > > }, > > > > > > "schema_version": { > > > > > > "type": "string" > > > > > > }, > > > > > > "aws_access_key": { > > > > > > "type": "string" > > > > > > }, > > > > > > "aws_secret_key": { > > > > > > "type": "string" > > > > > > } > > > > > > }, > > > > > > "required": [ > > > > > > "name", > > > > > > "schema_version", > > > > > > "aws_access_key", > > > > > > "aws_secret_key" > > > > > > ] > > > > > > } > > > > > > > > > > > > Fictional Connector config schema version:3.0 > > > > > > --- > > > > > > { > > > > > > "$schema": "http://json-schema.org/draft-04/schema#";, > > > > > > "type": "object", > > > > > > "properties": { > > > > > > "name": { > > > > > > "type": "string" > > > > > > }, > > > > > > "schema_version": { > > > > > > "type": "string" > > > > > > }, > > > > > > "iam_role": { > > > > > > "type": "string" > > > > > > } > > > > > > }, > > > > > > "required": [ > > > > > > "name", > > > > > > "schema_version", > > > > > > "iam_role" > > > > > > ] > > > > > > } > > > > > > > > > > > > The connector which supports Fictional config schema 2.0 will > > validate > > > > > the > > > > > > access key and secret key. > > > > > > Whereas a connector which supports config with schema version 3.0 > > will > > > > > only > > > > > > validate the IAM role. > > > > > > > > > > > > This is the alternative which I wanted to suggest. Each plugin will > > > > > > register the schema versions of connector config which it supports. > > > > > > > > > > > > The plugin paths may be optionally different i.e we don't have to > > > > > > mandatorily add a new plugin path to support a new schema version. > > > > > > > > > > > > Thanks, > > > > > > Ashwin > > > > > > > > > > > > On Tue, Nov 22, 2022 at 12:47 PM Snehashis < > > snehashisp1...@gmail.com> > > > > > > wrote: > > > > > > > > > > > > > Thanks for the input Ashwin. > > > > > > > > > > > > > > > 1. Can you elaborate on the rejected alternatives ? Suppose > > connector > > > > > > > > config is versioned and has a schema. Then a single plugin > > (whose > > > > > > > > dependencies have not changed) can handle multiple config > > versions > > > > > for > > > > > > > the > > > > > > > > same connector class. > > > > > > > > > > > > > > IIUC (please correct me if I am wrong here), what you highlighted > > > > > above, > > > > > > is > > > > > > > a versioning scheme for a connector config for the same > > connector (and > > > > > > not > > > > > > > different versions of a connector plugin). That is a somewhat > > > > > tangential > > > > > > > problem. While it is definitely a useful feature to have, like a > > log to > > > > > > > check what changes were made over time to the config which might > > make > > > > > it > > > > > > > easier to do rollbacks, it is not the focus here. Here by > > version we > > > > > mean > > > > > > > to say what underlying version of the plugin should the given > > > > > > configuration > > > > > > > of the connector use. Perhaps it is better to change the name of > > the > > > > > > > parameter from connector.version to connector.plugin.version or > > > > > > > plugin.version if it was confusing. wdyt? > > > > > > > > > > > > > > > 2. Any plans to support assisted migration e.g if a user > > invokes > > > > > "POST > > > > > > > > connector/config?migrate=latest", the latest version > > __attempts__ to > > > > > > > > transform the existing config to the newer version. This would > > > > > require > > > > > > > > adding a method like "boolean migrate(Version fromVersion)" to > > the > > > > > > > > connector interface. > > > > > > > > > > > > > > This is an enhancement we can think of doing in future. Users can > > > > > simply > > > > > > do > > > > > > > a PUT call with the updated config which has the updated version > > > > > number. > > > > > > > The assisted mode could be handy as the user does not need to > > know the > > > > > > > config but beyond this it does not seem to justify its existence. > > > > > > > > > > > > > > Regards > > > > > > > Snehashis > > > > > > > > > > > > > > On Tue, Nov 22, 2022 at 10:50 AM Ashwin > > <apan...@confluent.io.invalid> > > > > > > > wrote: > > > > > > > > > > > > > > > Hi Snehasis, > > > > > > > > > > > > > > > > This is a really useful feature and thanks for initiating this > > > > > > > discussion. > > > > > > > > > > > > > > > > I had the following questions - > > > > > > > > > > > > > > > > > > > > > > > > 1. Can you elaborate on the rejected alternatives ? Suppose > > connector > > > > > > > > config is versioned and has a schema. Then a single plugin > > (whose > > > > > > > > dependencies have not changed) can handle multiple config > > versions > > > > > for > > > > > > > the > > > > > > > > same connector class. > > > > > > > > > > > > > > > > 2. Any plans to support assisted migration e.g if a user > > invokes > > > > > "POST > > > > > > > > connector/config?migrate=latest", the latest version > > __attempts__ to > > > > > > > > transform the existing config to the newer version. This would > > > > > require > > > > > > > > adding a method like "boolean migrate(Version fromVersion)" to > > the > > > > > > > > connector interface. > > > > > > > > > > > > > > > > Thanks, > > > > > > > > Ashwin > > > > > > > > > > > > > > > > On Mon, Nov 21, 2022 at 2:27 PM Snehashis < > > snehashisp1...@gmail.com> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > I'd like to start a discussion thread on KIP-891: Running > > multiple > > > > > > > > versions > > > > > > > > > of a connector. > > > > > > > > > > > > > > > > > > The KIP aims to add the ability for the connect runtime to > > run > > > > > > multiple > > > > > > > > > versions of a connector. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-891%3A+Running+multiple+versions+of+a+connector > > > > > > > > > > > > > > > > > > Please take a look and let me know what you think. > > > > > > > > > > > > > > > > > > Thank you > > > > > > > > > Snehashis Pal > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
