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 > >> > > > > > > > >> > > > >> > > > > > > > >> > > >> > > > > > > > >> > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > > >
