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