I wasn't trying to suggest deprecating all verbs and endpoints prefixed with `/connector-plugins`, just the `GET: /connector-plugins` as I detailed in the KIP. Maybe I'm not understanding your recommendation, or there's a way I can make the KIP text clearer? Or do you disagree this is the right move tactically? I thought my text was sufficient: "it will be redundant to this new, more generally useful endpoint."
On Mon, Jun 7, 2021 at 12:56 PM Konstantine Karantasis <konstant...@confluent.io.invalid> wrote: > Thanks for the response Cyrus. > > If we are suggesting deprecation of `/connector-plugins` in favor of > `/plugins` I think we should mention explicitly that the new endpoint > covers the existing functionality precisely, including the validation > endpoint for connector plugins. > I'd recommend extending the spec to refer to this, adding an example to > make this clear and mentioning that the two endpoints will be equivalent > with respect to existing functionality in the upgrade section. > > Konstantine > > On Mon, Jun 7, 2021 at 10:57 AM Cyrus Vafadari <cvafad...@gmail.com> > wrote: > > > I will move to vote > > > > On Mon, Jun 7, 2021 at 10:44 AM Cyrus Vafadari <cvafad...@gmail.com> > > wrote: > > > > > Thanks for the feedback, > > > > > > I added an example request/response for SMTs, and I thought about your > > > suggestion re:deprecation and am now explicitly proposing to mark the > > > existing endpoint as deprecated, though I don't anticipate the need to > > > remove it will arise any time soon! > > > > > > Cyrus > > > > > > On Fri, Jun 4, 2021 at 7:35 PM Konstantine Karantasis > > > <konstant...@confluent.io.invalid> wrote: > > > > > >> Hi Cyrus. > > >> > > >> The proposal looks good and I like the API spec definition the way > it's > > >> presented. > > >> > > >> Having said that, a few examples that would list the request type and > > >> body, > > >> the returned status and the json response would be nice too, following > > the > > >> tradition of other KIPs. > > >> > > >> See > > >> > > >> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-745%3A+Connect+API+to+restart+connector+and+tasks > > >> for a recent example. > > >> > > >> I also see there's no mention regarding the future of the current > > >> `/connector-plugins` endpoint or any deprecation plans. > > >> > > >> I think we should make our intentions clear in the KIP itself > > >> which introduces the new endpoint as a superset of the old one, beyond > > the > > >> discussion in this thread, for future readers. > > >> Also, I think it's useful to keep in mind that deprecation doesn't > > >> necessarily mean imminent removal. The next opportunity to remove this > > end > > >> point would be the next major release at the earliest. > > >> > > >> Regards, > > >> Konstantine > > >> > > >> > > >> On Mon, Apr 19, 2021 at 10:13 AM Cyrus Vafadari <cvafad...@gmail.com> > > >> wrote: > > >> > > >> > Thanks! Anyone else from the community with final thoughts before > > going > > >> to > > >> > vote? > > >> > > > >> > On Mon, Apr 19, 2021 at 4:16 AM Tom Bentley <tbent...@redhat.com> > > >> wrote: > > >> > > > >> > > Hi Cyrus, > > >> > > > > >> > > That seems reasonable to me. > > >> > > > > >> > > On Thu, Apr 15, 2021 at 6:44 PM Cyrus Vafadari < > cvafad...@gmail.com > > > > > >> > > wrote: > > >> > > > > >> > > > What do you think of "type" field remaining in, and returning > > >> > > > "transformation", "converter" or whatever type it is. This way > the > > >> > schema > > >> > > > remains consistent, and you can programmatically understand what > > >> > plugins > > >> > > > are returned on the holistic "GET /plugins" endpoint? It will be > > >> > slightly > > >> > > > redundant in the case where you specify the plugin-type as a > path > > >> > > > parameter. > > >> > > > > > >> > > > On Thu, Apr 15, 2021 at 1:13 PM Tom Bentley < > tbent...@redhat.com> > > >> > wrote: > > >> > > > > > >> > > > > Hi Cyrus, > > >> > > > > > > >> > > > > Re 2: A very minor thing but while type=source|sink for a > > >> connector, > > >> > it > > >> > > > > doesn't makes sense for the other plugin types, but so the > json > > >> for > > >> > > those > > >> > > > > plugins should omit that property rather than have type=null. > > >> > > > > > > >> > > > > Apart from that it seems reasonable to me. Thanks again, > > >> > > > > > > >> > > > > Tom > > >> > > > > > > >> > > > > On Thu, Apr 15, 2021 at 3:49 PM Cyrus Vafadari < > > >> cvafad...@gmail.com> > > >> > > > > wrote: > > >> > > > > > > >> > > > > > Hi Tom, > > >> > > > > > > > >> > > > > > Thanks for taking the time to respond with thoughtful > > questions: > > >> > > > > > > > >> > > > > > 1. I propose HTTP-400, will update the KIP to reflect this > > >> proposal > > >> > > > > > 2. I think the schema should be of the same format. It is > > quite > > >> > > > minimal, > > >> > > > > so > > >> > > > > > there is room to add fields in the future without breaking > > >> > > > compatibility. > > >> > > > > > I will update the KIP to specify. > > >> > > > > > 3. Yes, that's right. > > >> > > > > > 4. I personally don't see the value in deprecating since > it's > > a > > >> > > simple > > >> > > > > > alias (interface and impl will both be simple changes). I > > would > > >> be > > >> > > > > > comfortable kicking that can down the road if/when there is > a > > >> need > > >> > > for > > >> > > > an > > >> > > > > > actual breaking change. That way we can keep the scope and > > diff > > >> > here > > >> > > > > tight. > > >> > > > > > But iff the community feels strongly that deprecating is the > > >> right > > >> > > > thing > > >> > > > > to > > >> > > > > > do, I'm happy to update the KIP to propose deprecating. > > >> > > > > > > > >> > > > > > Thanks again! > > >> > > > > > > > >> > > > > > On Wed, Apr 14, 2021 at 9:38 AM Tom Bentley < > > >> tbent...@redhat.com> > > >> > > > wrote: > > >> > > > > > > > >> > > > > > > Hi Cyrus, > > >> > > > > > > > > >> > > > > > > Thanks for the KIP. A few questions: > > >> > > > > > > > > >> > > > > > > 1. What status code does the /plugins/{plugin-type} > endpoint > > >> > return > > >> > > > > when > > >> > > > > > an > > >> > > > > > > unknown type is given? > > >> > > > > > > 2. The result of /connector-plugins is a list of objects > > with > > >> > > > 'class', > > >> > > > > > > 'type' and 'version' properties. Presumably > > >> /plugins/connector is > > >> > > the > > >> > > > > > same, > > >> > > > > > > but what is the schema for the other plugin types? > > >> > > > > > > 3. You're not proposing an equivalent of the > > >> > > > > > > /connector-plugins/{connectorType}/config/validate > endpoint > > >> for > > >> > > > > > > non-connector types, which I think makes sense, because > you > > >> would > > >> > > > > > validate > > >> > > > > > > an SMT's config via its used by a connector, so the > existing > > >> > > endpoint > > >> > > > > > > suffices, right? > > >> > > > > > > 4. Would /connector-plugins eventually be deprecated in > > >> favour of > > >> > > > > > > /plugins/connector, or do we expect it to remain in the > API > > >> > > > > indefinitely > > >> > > > > > > and accept that /connector-plugins and /plugins/connector > > >> provide > > >> > > > > > identical > > >> > > > > > > responses? If we had the intention to deprecate in the > > future > > >> > maybe > > >> > > > we > > >> > > > > > > should add a /plugins/connector/config/validate endpoint > > now? > > >> > > > > > > > > >> > > > > > > Many thanks, > > >> > > > > > > > > >> > > > > > > Tom > > >> > > > > > > > > >> > > > > > > On Tue, Apr 13, 2021 at 6:46 PM Cyrus Vafadari < > > >> > > cvafad...@gmail.com> > > >> > > > > > > wrote: > > >> > > > > > > > > >> > > > > > > > Hello all, > > >> > > > > > > > > > >> > > > > > > > As the number of connect plugins, SMT's, etc have > grown, I > > >> > wanted > > >> > > > to > > >> > > > > > bump > > >> > > > > > > > this thread to see if there is more interest in adding a > > >> > Connect > > >> > > > REST > > >> > > > > > > > endpoint to get the current plugins in the worker. > > >> > > > > > > > > > >> > > > > > > > Thanks all, and thanks Chris for the initial feedback on > > >> this. > > >> > > > > > > > > > >> > > > > > > > On Mon, Feb 17, 2020 at 12:56 PM Cyrus Vafadari < > > >> > > > cvafad...@gmail.com > > >> > > > > > > > >> > > > > > > > wrote: > > >> > > > > > > > > > >> > > > > > > > > Thanks for the feedback, Chris. > > >> > > > > > > > > > > >> > > > > > > > > WRT Worker-plugins, I see your point that they aren't > > very > > >> > > useful > > >> > > > > for > > >> > > > > > > > > people trying to create connectors to see what exists > > >> > already, > > >> > > > but > > >> > > > > I > > >> > > > > > > > would > > >> > > > > > > > > find them useful for things like making assertions > > >> against a > > >> > > > docker > > >> > > > > > > image > > >> > > > > > > > > to confirm the pluginpaths/classpaths are configured > > >> > correctly. > > >> > > > It > > >> > > > > is > > >> > > > > > > > more > > >> > > > > > > > > useful for "sanity check" kinds of things than > > >> > > > exploring/browsing. > > >> > > > > > That > > >> > > > > > > > > said, I don't feel too strongly and if more feedback > > from > > >> the > > >> > > > > > community > > >> > > > > > > > > thinks they are better excluded then we can remove > them. > > >> > > > > > > > > > > >> > > > > > > > > WRT camelCase vs hyphens, I see your point here, will > > >> update > > >> > > the > > >> > > > > KIP. > > >> > > > > > > > > > > >> > > > > > > > > WRT compatibility -- Good question -- I would expect > > that > > >> > yes, > > >> > > > new > > >> > > > > > > > plugins > > >> > > > > > > > > would match this format. I will add this explicitly in > > the > > >> > KIP > > >> > > > for > > >> > > > > > > > clarity. > > >> > > > > > > > > I do think it's a valuable, simple feature to have the > > >> worker > > >> > > > able > > >> > > > > to > > >> > > > > > > > > report new plugins. > > >> > > > > > > > > > > >> > > > > > > > > Thanks again! > > >> > > > > > > > > > > >> > > > > > > > > On Wed, Feb 12, 2020 at 4:44 PM Christopher Egerton < > > >> > > > > > > chr...@confluent.io > > >> > > > > > > > > > > >> > > > > > > > > wrote: > > >> > > > > > > > > > > >> > > > > > > > >> Hi Cyrus, > > >> > > > > > > > >> > > >> > > > > > > > >> Thanks for the KIP! > > >> > > > > > > > >> > > >> > > > > > > > >> One quick question--I see the use case for being able > > to > > >> > list > > >> > > > > > > > >> per-connector > > >> > > > > > > > >> plugins (SMTs, converters, and header converters) via > > >> REST > > >> > > API, > > >> > > > > but > > >> > > > > > > I'm > > >> > > > > > > > >> not > > >> > > > > > > > >> so sure about worker plugins (REST extensions and > > config > > >> > > > > providers). > > >> > > > > > > > Since > > >> > > > > > > > >> those are configured at startup for the worker, is > > there > > >> any > > >> > > > > > advantage > > >> > > > > > > > to > > >> > > > > > > > >> being able to see the worker plugins available on a > > >> running > > >> > > > > worker? > > >> > > > > > > It's > > >> > > > > > > > >> not like they can be added during the lifetime of > that > > >> > worker, > > >> > > > and > > >> > > > > > > that > > >> > > > > > > > >> information wouldn't be useful to anyone trying to > > >> create a > > >> > > > > > connector > > >> > > > > > > as > > >> > > > > > > > >> they wouldn't be able to include worker plugins in a > > >> > connector > > >> > > > > > config > > >> > > > > > > > >> anyways. > > >> > > > > > > > >> > > >> > > > > > > > >> And a small nit--it seems like the precedent set > > >> > > > ever-so-slightly > > >> > > > > by > > >> > > > > > > the > > >> > > > > > > > >> /connector-plugins resource is to use a hyphen to > > >> separate a > > >> > > > > series > > >> > > > > > of > > >> > > > > > > > >> lowercase words in an endpoint path as opposed to > camel > > >> case > > >> > > > with > > >> > > > > > > > >> capitalization. What do you think about changing the > > >> > possible > > >> > > > > plugin > > >> > > > > > > > types > > >> > > > > > > > >> to "connector", "converter", "header-converter", > etc.? > > >> > > > > > > > >> > > >> > > > > > > > >> One final question about forwards compatibility--the > > >> > > description > > >> > > > > for > > >> > > > > > > the > > >> > > > > > > > >> /plugins endpoint indicates that it "Returns plugin > > >> > > descriptions > > >> > > > > of > > >> > > > > > > all > > >> > > > > > > > >> types". If another type of pluggable component is > added > > >> to > > >> > the > > >> > > > > > > > framework, > > >> > > > > > > > >> should we expect this endpoint to be updated to > include > > >> that > > >> > > > type > > >> > > > > of > > >> > > > > > > > >> plugin > > >> > > > > > > > >> as well? And if so, should we also expect a matching > > >> > > > > > > > /plugins/{pluginType} > > >> > > > > > > > >> endpoint to be added? > > >> > > > > > > > >> > > >> > > > > > > > >> Cheers, > > >> > > > > > > > >> > > >> > > > > > > > >> Chris > > >> > > > > > > > >> > > >> > > > > > > > >> On Fri, Sep 6, 2019 at 11:48 AM Cyrus Vafadari < > > >> > > > > cy...@confluent.io> > > >> > > > > > > > >> wrote: > > >> > > > > > > > >> > > >> > > > > > > > >> > I've updated the KIP here to simplify and clarify > the > > >> goal > > >> > > -- > > >> > > > > > it's a > > >> > > > > > > > >> pretty > > >> > > > > > > > >> > simple KIP to add a REST endpoint for SMTs and > other > > >> > > plugins. > > >> > > > > This > > >> > > > > > > > will > > >> > > > > > > > >> > make it easier to see what SMTs you've loaded. > > >> > > > > > > > >> > > > >> > > > > > > > >> > Hoping for some good discussion! > > >> > > > > > > > >> > > > >> > > > > > > > >> > Thanks, all > > >> > > > > > > > >> > > > >> > > > > > > > >> > On Sat, Jul 20, 2019 at 9:07 PM Cyrus Vafadari < > > >> > > > > > cy...@confluent.io> > > >> > > > > > > > >> wrote: > > >> > > > > > > > >> > > > >> > > > > > > > >> > > Hello, all, > > >> > > > > > > > >> > > > > >> > > > > > > > >> > > I'd like to start discussion on a new KIP I'm > > >> proposing > > >> > to > > >> > > > add > > >> > > > > > > HTTP > > >> > > > > > > > >> > > endpoints to Kafka Connect to give us some more > > >> insight > > >> > > into > > >> > > > > > other > > >> > > > > > > > >> > > non-connector plugins, like Simple Message > > >> > Transformations > > >> > > > > > (SMTs). > > >> > > > > > > > The > > >> > > > > > > > >> > KIP > > >> > > > > > > > >> > > would not change how Connect works in any > > meaningful > > >> way > > >> > > > > except > > >> > > > > > to > > >> > > > > > > > >> expose > > >> > > > > > > > >> > > some more data by the endpoint, analogous to > > existing > > >> > > > > > > > >> > > "ConnectorPluginsResource" class. > > >> > > > > > > > >> > > > > >> > > > > > > > >> > > Looking forward to getting some feedback. > > >> > > > > > > > >> > > > > >> > > > > > > > >> > > Cyrus > > >> > > > > > > > >> > > > > >> > > > > > > > >> > > > > >> > > > > > > > >> > > > > >> > > > > > > > >> > > > >> > > > > > > > >> > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-494%3A+Connect+REST+Endpoint+for+Transformations+%28SMTs%29+and+other+Plugins > > >> > > > > > > > >> > > > > >> > > > > > > > >> > > > >> > > > > > > > >> > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > > > > >
