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