That's a valid point based on what the implementation is today. But this
new endpoint aims to be an extension and a superset in terms of
functionality of the existing `/connector-plugins` endpoint.
This last discussion and the fact that the endpoint `/plugins/connector`
that you suggest is almost synonymous to `/connector-plugins' makes me
think that we might have not thought through how this new endpoint will
evolve and look as a top level endpoint for the various plugin types.
Konstantine
On Mon, Jun 7, 2021 at 5:02 PM Cyrus Vafadari <cvafad...@gmail.com> wrote:
> Since it is just two endpoints, I agree it's fine to move them over.
>
> However, the endpoint you suggest would imply that someone could attempt to
> validate the configs of an SMT or Header Converter, since it doesn't
> specify the plugin's type. So if you agree I'll update the proposal to move
> the PUT endpoint to
>
> PUT /plugins/connector/{connector-type}/config/validate
>
> so we don't have to return 400s for all the other types for which
> validation is not supported.
>
> Does that sound good to you?
>
>
> On Mon, Jun 7, 2021 at 4:49 PM Konstantine Karantasis
> <konstant...@confluent.io.invalid> wrote:
>
> > Hi Cyrus,
> >
> > Given that `/connector-plugins` is a top level endpoint, I think the
> > suggestion to deprecate `/connector-plugins` should affect all the nested
> > endpoints.
> >
> > This should be straightforward here (in terms of KIP changes but also
> > implementation), because the endpoints are only two:
> >
> > GET /connector-plugins
> > and
> > PUT /connector-plugins/{connector-type}/config/validate
> >
> > I find deprecating both of them in favor of:
> > GET /plugins
> > and
> > PUT /plugins/{connector-type}/config/validate
> >
> > more clear and aligned with how we want the new endpoint to look like.
> >
> > Wdyt?
> >
> > Konstantine
> >
> >
> > On Mon, Jun 7, 2021 at 4:18 PM Cyrus Vafadari <cvafad...@gmail.com>
> wrote:
> >
> > > 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
> > > > > >> > > > > > > > >> > >
> > > > > >> > > > > > > > >> >
> > > > > >> > > > > > > > >>
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > >
> > > > > >> > > > > > >
> > > > > >> > > > > >
> > > > > >> > > > >
> > > > > >> > > >
> > > > > >> > >
> > > > > >> >
> > > > > >>
> > > > > >
> > > > >
> > > >
> > >
> >
>
