You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Ted Yu <yu...@gmail.com> on 2017/09/08 17:00:34 UTC
[DISCUSS] KIP-197: Include Connector type in Connector REST API
Hi,
Please take a look at:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-197+Connect+REST+API+should+include+the+connector+type+when+describing+a+connector
Thanks
Re: [DISCUSS] KIP-197: Include Connector type in Connector REST API
Posted by Ismael Juma <is...@juma.me.uk>.
Thanks for the KIP, +1 (binding).
Ismael
On Fri, Sep 8, 2017 at 6:00 PM, Ted Yu <yu...@gmail.com> wrote:
> Hi,
> Please take a look at:
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 197+Connect+REST+API+should+include+the+connector+type+
> when+describing+a+connector
>
> Thanks
>
Re: [DISCUSS] KIP-197: Include Connector type in Connector REST API
Posted by Ewen Cheslack-Postava <ew...@confluent.io>.
Awesome turnaround time :) Trying to get another committer to take a look
so we can get this in for the next release.
-Ewen
On Mon, Sep 11, 2017 at 8:30 PM, Ted Yu <yu...@gmail.com> wrote:
> Updated KIP-197 with the reference to KIP-151
>
> On Mon, Sep 11, 2017 at 8:24 PM, Ewen Cheslack-Postava <ew...@confluent.io>
> wrote:
>
> > Yeah, this all seems reasonable to me as well. Even if there are other
> > places we should add this info, these seem like the really useful ones.
> >
> > re: the enum generating lowercase, that came in KIP-151 (which was the
> > first to expose this info, and technically is enough to not *require*
> this
> > change, but this change is still simpler/easier than having to do
> multiple
> > calls). Might want to reference that in the KIP just for reference and
> call
> > out specifically that the enum already does lowercase so we will always
> do
> > that, but for compatibility, consumers of the api should probably not
> make
> > assumptions about the capitalization.
> >
> > -ewen
> >
> > On Fri, Sep 8, 2017 at 4:24 PM, Randall Hauch <rh...@gmail.com> wrote:
> >
> > > Looks good to me! Thanks again!
> > >
> > > I say go ahead and ask for a vote in a new thread.
> > >
> > > Randall
> > >
> > > On Fri, Sep 8, 2017 at 6:22 PM, Ted Yu <yu...@gmail.com> wrote:
> > >
> > > > Lowercase constants are generated by the enum.
> > > >
> > > > Updated KIP again.
> > > >
> > > > On Fri, Sep 8, 2017 at 4:08 PM, Randall Hauch <rh...@gmail.com>
> > wrote:
> > > >
> > > > > One more thing. I wonder if we should use lowercase constants for
> the
> > > > types
> > > > > rather than mixed case: "sink" and "source" (rather than "Sink" and
> > > > > "Source"). Thoughts?
> > > > >
> > > > > On Fri, Sep 8, 2017 at 6:02 PM, Ted Yu <yu...@gmail.com>
> wrote:
> > > > >
> > > > > > Updated the KIP accordingly.
> > > > > >
> > > > > > Cheers
> > > > > >
> > > > > > On Fri, Sep 8, 2017 at 3:57 PM, Randall Hauch <rh...@gmail.com>
> > > > wrote:
> > > > > >
> > > > > > > Hi, Ted. Thanks for the quick turn around.
> > > > > > >
> > > > > > > I didn't remember that the response to the
> > > > "/connectors/{name}/config"
> > > > > is
> > > > > > > the actual configuration, so we probably shouldn't add "type"
> > > there.
> > > > > > Sorry
> > > > > > > about that.
> > > > > > >
> > > > > > > And in the "/connector/{name}/status" response, I wonder if it
> > > would
> > > > be
> > > > > > > better to embed the "type" within the "connector" field, which
> > > > > > corresponds
> > > > > > > to adding the type to the `ConnectorStateInfo` (just like you
> did
> > > to
> > > > > the
> > > > > > > `ConnectorInfo` class). WDYT?
> > > > > > >
> > > > > > > Best regards,
> > > > > > >
> > > > > > > Randall
> > > > > > >
> > > > > > > On Fri, Sep 8, 2017 at 1:12 PM, Ted Yu <yu...@gmail.com>
> > > wrote:
> > > > > > >
> > > > > > > > Thanks for the reminder, Randall.
> > > > > > > >
> > > > > > > > I have modified the KIP to include these two endpoints.
> > > > > > > >
> > > > > > > > On Fri, Sep 8, 2017 at 11:00 AM, Randall Hauch <
> > rhauch@gmail.com
> > > >
> > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi, Ted.
> > > > > > > > >
> > > > > > > > > Thanks for creating this KIP and for working on the
> > > > implementation.
> > > > > > The
> > > > > > > > > proposal looks great for the "/connectors/{name}" endpoint,
> > but
> > > > > there
> > > > > > > are
> > > > > > > > > several others that we need to consider as well so that the
> > > > > responses
> > > > > > > are
> > > > > > > > > all consistent. In particular, look at
> > > > "/connectors/{name}/status"
> > > > > > and
> > > > > > > > > "/connectors/{name}/config" (at least) that include the
> > > connector
> > > > > > > > > information.
> > > > > > > > >
> > > > > > > > > Best regards,
> > > > > > > > >
> > > > > > > > > Randall
> > > > > > > > >
> > > > > > > > > On Fri, Sep 8, 2017 at 12:00 PM, Ted Yu <
> yuzhihong@gmail.com
> > >
> > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Hi,
> > > > > > > > > > Please take a look at:
> > > > > > > > > >
> > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > > > > > 197+Connect+REST+API+should+include+the+connector+type+
> > > > > > > > > > when+describing+a+connector
> > > > > > > > > >
> > > > > > > > > > Thanks
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-197: Include Connector type in Connector REST API
Posted by Ted Yu <yu...@gmail.com>.
Updated KIP-197 with the reference to KIP-151
On Mon, Sep 11, 2017 at 8:24 PM, Ewen Cheslack-Postava <ew...@confluent.io>
wrote:
> Yeah, this all seems reasonable to me as well. Even if there are other
> places we should add this info, these seem like the really useful ones.
>
> re: the enum generating lowercase, that came in KIP-151 (which was the
> first to expose this info, and technically is enough to not *require* this
> change, but this change is still simpler/easier than having to do multiple
> calls). Might want to reference that in the KIP just for reference and call
> out specifically that the enum already does lowercase so we will always do
> that, but for compatibility, consumers of the api should probably not make
> assumptions about the capitalization.
>
> -ewen
>
> On Fri, Sep 8, 2017 at 4:24 PM, Randall Hauch <rh...@gmail.com> wrote:
>
> > Looks good to me! Thanks again!
> >
> > I say go ahead and ask for a vote in a new thread.
> >
> > Randall
> >
> > On Fri, Sep 8, 2017 at 6:22 PM, Ted Yu <yu...@gmail.com> wrote:
> >
> > > Lowercase constants are generated by the enum.
> > >
> > > Updated KIP again.
> > >
> > > On Fri, Sep 8, 2017 at 4:08 PM, Randall Hauch <rh...@gmail.com>
> wrote:
> > >
> > > > One more thing. I wonder if we should use lowercase constants for the
> > > types
> > > > rather than mixed case: "sink" and "source" (rather than "Sink" and
> > > > "Source"). Thoughts?
> > > >
> > > > On Fri, Sep 8, 2017 at 6:02 PM, Ted Yu <yu...@gmail.com> wrote:
> > > >
> > > > > Updated the KIP accordingly.
> > > > >
> > > > > Cheers
> > > > >
> > > > > On Fri, Sep 8, 2017 at 3:57 PM, Randall Hauch <rh...@gmail.com>
> > > wrote:
> > > > >
> > > > > > Hi, Ted. Thanks for the quick turn around.
> > > > > >
> > > > > > I didn't remember that the response to the
> > > "/connectors/{name}/config"
> > > > is
> > > > > > the actual configuration, so we probably shouldn't add "type"
> > there.
> > > > > Sorry
> > > > > > about that.
> > > > > >
> > > > > > And in the "/connector/{name}/status" response, I wonder if it
> > would
> > > be
> > > > > > better to embed the "type" within the "connector" field, which
> > > > > corresponds
> > > > > > to adding the type to the `ConnectorStateInfo` (just like you did
> > to
> > > > the
> > > > > > `ConnectorInfo` class). WDYT?
> > > > > >
> > > > > > Best regards,
> > > > > >
> > > > > > Randall
> > > > > >
> > > > > > On Fri, Sep 8, 2017 at 1:12 PM, Ted Yu <yu...@gmail.com>
> > wrote:
> > > > > >
> > > > > > > Thanks for the reminder, Randall.
> > > > > > >
> > > > > > > I have modified the KIP to include these two endpoints.
> > > > > > >
> > > > > > > On Fri, Sep 8, 2017 at 11:00 AM, Randall Hauch <
> rhauch@gmail.com
> > >
> > > > > wrote:
> > > > > > >
> > > > > > > > Hi, Ted.
> > > > > > > >
> > > > > > > > Thanks for creating this KIP and for working on the
> > > implementation.
> > > > > The
> > > > > > > > proposal looks great for the "/connectors/{name}" endpoint,
> but
> > > > there
> > > > > > are
> > > > > > > > several others that we need to consider as well so that the
> > > > responses
> > > > > > are
> > > > > > > > all consistent. In particular, look at
> > > "/connectors/{name}/status"
> > > > > and
> > > > > > > > "/connectors/{name}/config" (at least) that include the
> > connector
> > > > > > > > information.
> > > > > > > >
> > > > > > > > Best regards,
> > > > > > > >
> > > > > > > > Randall
> > > > > > > >
> > > > > > > > On Fri, Sep 8, 2017 at 12:00 PM, Ted Yu <yuzhihong@gmail.com
> >
> > > > wrote:
> > > > > > > >
> > > > > > > > > Hi,
> > > > > > > > > Please take a look at:
> > > > > > > > >
> > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > > > > 197+Connect+REST+API+should+include+the+connector+type+
> > > > > > > > > when+describing+a+connector
> > > > > > > > >
> > > > > > > > > Thanks
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-197: Include Connector type in Connector REST API
Posted by Ewen Cheslack-Postava <ew...@confluent.io>.
Yeah, this all seems reasonable to me as well. Even if there are other
places we should add this info, these seem like the really useful ones.
re: the enum generating lowercase, that came in KIP-151 (which was the
first to expose this info, and technically is enough to not *require* this
change, but this change is still simpler/easier than having to do multiple
calls). Might want to reference that in the KIP just for reference and call
out specifically that the enum already does lowercase so we will always do
that, but for compatibility, consumers of the api should probably not make
assumptions about the capitalization.
-ewen
On Fri, Sep 8, 2017 at 4:24 PM, Randall Hauch <rh...@gmail.com> wrote:
> Looks good to me! Thanks again!
>
> I say go ahead and ask for a vote in a new thread.
>
> Randall
>
> On Fri, Sep 8, 2017 at 6:22 PM, Ted Yu <yu...@gmail.com> wrote:
>
> > Lowercase constants are generated by the enum.
> >
> > Updated KIP again.
> >
> > On Fri, Sep 8, 2017 at 4:08 PM, Randall Hauch <rh...@gmail.com> wrote:
> >
> > > One more thing. I wonder if we should use lowercase constants for the
> > types
> > > rather than mixed case: "sink" and "source" (rather than "Sink" and
> > > "Source"). Thoughts?
> > >
> > > On Fri, Sep 8, 2017 at 6:02 PM, Ted Yu <yu...@gmail.com> wrote:
> > >
> > > > Updated the KIP accordingly.
> > > >
> > > > Cheers
> > > >
> > > > On Fri, Sep 8, 2017 at 3:57 PM, Randall Hauch <rh...@gmail.com>
> > wrote:
> > > >
> > > > > Hi, Ted. Thanks for the quick turn around.
> > > > >
> > > > > I didn't remember that the response to the
> > "/connectors/{name}/config"
> > > is
> > > > > the actual configuration, so we probably shouldn't add "type"
> there.
> > > > Sorry
> > > > > about that.
> > > > >
> > > > > And in the "/connector/{name}/status" response, I wonder if it
> would
> > be
> > > > > better to embed the "type" within the "connector" field, which
> > > > corresponds
> > > > > to adding the type to the `ConnectorStateInfo` (just like you did
> to
> > > the
> > > > > `ConnectorInfo` class). WDYT?
> > > > >
> > > > > Best regards,
> > > > >
> > > > > Randall
> > > > >
> > > > > On Fri, Sep 8, 2017 at 1:12 PM, Ted Yu <yu...@gmail.com>
> wrote:
> > > > >
> > > > > > Thanks for the reminder, Randall.
> > > > > >
> > > > > > I have modified the KIP to include these two endpoints.
> > > > > >
> > > > > > On Fri, Sep 8, 2017 at 11:00 AM, Randall Hauch <rhauch@gmail.com
> >
> > > > wrote:
> > > > > >
> > > > > > > Hi, Ted.
> > > > > > >
> > > > > > > Thanks for creating this KIP and for working on the
> > implementation.
> > > > The
> > > > > > > proposal looks great for the "/connectors/{name}" endpoint, but
> > > there
> > > > > are
> > > > > > > several others that we need to consider as well so that the
> > > responses
> > > > > are
> > > > > > > all consistent. In particular, look at
> > "/connectors/{name}/status"
> > > > and
> > > > > > > "/connectors/{name}/config" (at least) that include the
> connector
> > > > > > > information.
> > > > > > >
> > > > > > > Best regards,
> > > > > > >
> > > > > > > Randall
> > > > > > >
> > > > > > > On Fri, Sep 8, 2017 at 12:00 PM, Ted Yu <yu...@gmail.com>
> > > wrote:
> > > > > > >
> > > > > > > > Hi,
> > > > > > > > Please take a look at:
> > > > > > > >
> > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > > > 197+Connect+REST+API+should+include+the+connector+type+
> > > > > > > > when+describing+a+connector
> > > > > > > >
> > > > > > > > Thanks
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-197: Include Connector type in Connector REST API
Posted by Randall Hauch <rh...@gmail.com>.
Looks good to me! Thanks again!
I say go ahead and ask for a vote in a new thread.
Randall
On Fri, Sep 8, 2017 at 6:22 PM, Ted Yu <yu...@gmail.com> wrote:
> Lowercase constants are generated by the enum.
>
> Updated KIP again.
>
> On Fri, Sep 8, 2017 at 4:08 PM, Randall Hauch <rh...@gmail.com> wrote:
>
> > One more thing. I wonder if we should use lowercase constants for the
> types
> > rather than mixed case: "sink" and "source" (rather than "Sink" and
> > "Source"). Thoughts?
> >
> > On Fri, Sep 8, 2017 at 6:02 PM, Ted Yu <yu...@gmail.com> wrote:
> >
> > > Updated the KIP accordingly.
> > >
> > > Cheers
> > >
> > > On Fri, Sep 8, 2017 at 3:57 PM, Randall Hauch <rh...@gmail.com>
> wrote:
> > >
> > > > Hi, Ted. Thanks for the quick turn around.
> > > >
> > > > I didn't remember that the response to the
> "/connectors/{name}/config"
> > is
> > > > the actual configuration, so we probably shouldn't add "type" there.
> > > Sorry
> > > > about that.
> > > >
> > > > And in the "/connector/{name}/status" response, I wonder if it would
> be
> > > > better to embed the "type" within the "connector" field, which
> > > corresponds
> > > > to adding the type to the `ConnectorStateInfo` (just like you did to
> > the
> > > > `ConnectorInfo` class). WDYT?
> > > >
> > > > Best regards,
> > > >
> > > > Randall
> > > >
> > > > On Fri, Sep 8, 2017 at 1:12 PM, Ted Yu <yu...@gmail.com> wrote:
> > > >
> > > > > Thanks for the reminder, Randall.
> > > > >
> > > > > I have modified the KIP to include these two endpoints.
> > > > >
> > > > > On Fri, Sep 8, 2017 at 11:00 AM, Randall Hauch <rh...@gmail.com>
> > > wrote:
> > > > >
> > > > > > Hi, Ted.
> > > > > >
> > > > > > Thanks for creating this KIP and for working on the
> implementation.
> > > The
> > > > > > proposal looks great for the "/connectors/{name}" endpoint, but
> > there
> > > > are
> > > > > > several others that we need to consider as well so that the
> > responses
> > > > are
> > > > > > all consistent. In particular, look at
> "/connectors/{name}/status"
> > > and
> > > > > > "/connectors/{name}/config" (at least) that include the connector
> > > > > > information.
> > > > > >
> > > > > > Best regards,
> > > > > >
> > > > > > Randall
> > > > > >
> > > > > > On Fri, Sep 8, 2017 at 12:00 PM, Ted Yu <yu...@gmail.com>
> > wrote:
> > > > > >
> > > > > > > Hi,
> > > > > > > Please take a look at:
> > > > > > >
> > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > > 197+Connect+REST+API+should+include+the+connector+type+
> > > > > > > when+describing+a+connector
> > > > > > >
> > > > > > > Thanks
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-197: Include Connector type in Connector REST API
Posted by Ted Yu <yu...@gmail.com>.
Lowercase constants are generated by the enum.
Updated KIP again.
On Fri, Sep 8, 2017 at 4:08 PM, Randall Hauch <rh...@gmail.com> wrote:
> One more thing. I wonder if we should use lowercase constants for the types
> rather than mixed case: "sink" and "source" (rather than "Sink" and
> "Source"). Thoughts?
>
> On Fri, Sep 8, 2017 at 6:02 PM, Ted Yu <yu...@gmail.com> wrote:
>
> > Updated the KIP accordingly.
> >
> > Cheers
> >
> > On Fri, Sep 8, 2017 at 3:57 PM, Randall Hauch <rh...@gmail.com> wrote:
> >
> > > Hi, Ted. Thanks for the quick turn around.
> > >
> > > I didn't remember that the response to the "/connectors/{name}/config"
> is
> > > the actual configuration, so we probably shouldn't add "type" there.
> > Sorry
> > > about that.
> > >
> > > And in the "/connector/{name}/status" response, I wonder if it would be
> > > better to embed the "type" within the "connector" field, which
> > corresponds
> > > to adding the type to the `ConnectorStateInfo` (just like you did to
> the
> > > `ConnectorInfo` class). WDYT?
> > >
> > > Best regards,
> > >
> > > Randall
> > >
> > > On Fri, Sep 8, 2017 at 1:12 PM, Ted Yu <yu...@gmail.com> wrote:
> > >
> > > > Thanks for the reminder, Randall.
> > > >
> > > > I have modified the KIP to include these two endpoints.
> > > >
> > > > On Fri, Sep 8, 2017 at 11:00 AM, Randall Hauch <rh...@gmail.com>
> > wrote:
> > > >
> > > > > Hi, Ted.
> > > > >
> > > > > Thanks for creating this KIP and for working on the implementation.
> > The
> > > > > proposal looks great for the "/connectors/{name}" endpoint, but
> there
> > > are
> > > > > several others that we need to consider as well so that the
> responses
> > > are
> > > > > all consistent. In particular, look at "/connectors/{name}/status"
> > and
> > > > > "/connectors/{name}/config" (at least) that include the connector
> > > > > information.
> > > > >
> > > > > Best regards,
> > > > >
> > > > > Randall
> > > > >
> > > > > On Fri, Sep 8, 2017 at 12:00 PM, Ted Yu <yu...@gmail.com>
> wrote:
> > > > >
> > > > > > Hi,
> > > > > > Please take a look at:
> > > > > >
> > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > 197+Connect+REST+API+should+include+the+connector+type+
> > > > > > when+describing+a+connector
> > > > > >
> > > > > > Thanks
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-197: Include Connector type in Connector REST API
Posted by Randall Hauch <rh...@gmail.com>.
One more thing. I wonder if we should use lowercase constants for the types
rather than mixed case: "sink" and "source" (rather than "Sink" and
"Source"). Thoughts?
On Fri, Sep 8, 2017 at 6:02 PM, Ted Yu <yu...@gmail.com> wrote:
> Updated the KIP accordingly.
>
> Cheers
>
> On Fri, Sep 8, 2017 at 3:57 PM, Randall Hauch <rh...@gmail.com> wrote:
>
> > Hi, Ted. Thanks for the quick turn around.
> >
> > I didn't remember that the response to the "/connectors/{name}/config" is
> > the actual configuration, so we probably shouldn't add "type" there.
> Sorry
> > about that.
> >
> > And in the "/connector/{name}/status" response, I wonder if it would be
> > better to embed the "type" within the "connector" field, which
> corresponds
> > to adding the type to the `ConnectorStateInfo` (just like you did to the
> > `ConnectorInfo` class). WDYT?
> >
> > Best regards,
> >
> > Randall
> >
> > On Fri, Sep 8, 2017 at 1:12 PM, Ted Yu <yu...@gmail.com> wrote:
> >
> > > Thanks for the reminder, Randall.
> > >
> > > I have modified the KIP to include these two endpoints.
> > >
> > > On Fri, Sep 8, 2017 at 11:00 AM, Randall Hauch <rh...@gmail.com>
> wrote:
> > >
> > > > Hi, Ted.
> > > >
> > > > Thanks for creating this KIP and for working on the implementation.
> The
> > > > proposal looks great for the "/connectors/{name}" endpoint, but there
> > are
> > > > several others that we need to consider as well so that the responses
> > are
> > > > all consistent. In particular, look at "/connectors/{name}/status"
> and
> > > > "/connectors/{name}/config" (at least) that include the connector
> > > > information.
> > > >
> > > > Best regards,
> > > >
> > > > Randall
> > > >
> > > > On Fri, Sep 8, 2017 at 12:00 PM, Ted Yu <yu...@gmail.com> wrote:
> > > >
> > > > > Hi,
> > > > > Please take a look at:
> > > > >
> > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > 197+Connect+REST+API+should+include+the+connector+type+
> > > > > when+describing+a+connector
> > > > >
> > > > > Thanks
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-197: Include Connector type in Connector REST API
Posted by Ted Yu <yu...@gmail.com>.
Updated the KIP accordingly.
Cheers
On Fri, Sep 8, 2017 at 3:57 PM, Randall Hauch <rh...@gmail.com> wrote:
> Hi, Ted. Thanks for the quick turn around.
>
> I didn't remember that the response to the "/connectors/{name}/config" is
> the actual configuration, so we probably shouldn't add "type" there. Sorry
> about that.
>
> And in the "/connector/{name}/status" response, I wonder if it would be
> better to embed the "type" within the "connector" field, which corresponds
> to adding the type to the `ConnectorStateInfo` (just like you did to the
> `ConnectorInfo` class). WDYT?
>
> Best regards,
>
> Randall
>
> On Fri, Sep 8, 2017 at 1:12 PM, Ted Yu <yu...@gmail.com> wrote:
>
> > Thanks for the reminder, Randall.
> >
> > I have modified the KIP to include these two endpoints.
> >
> > On Fri, Sep 8, 2017 at 11:00 AM, Randall Hauch <rh...@gmail.com> wrote:
> >
> > > Hi, Ted.
> > >
> > > Thanks for creating this KIP and for working on the implementation. The
> > > proposal looks great for the "/connectors/{name}" endpoint, but there
> are
> > > several others that we need to consider as well so that the responses
> are
> > > all consistent. In particular, look at "/connectors/{name}/status" and
> > > "/connectors/{name}/config" (at least) that include the connector
> > > information.
> > >
> > > Best regards,
> > >
> > > Randall
> > >
> > > On Fri, Sep 8, 2017 at 12:00 PM, Ted Yu <yu...@gmail.com> wrote:
> > >
> > > > Hi,
> > > > Please take a look at:
> > > >
> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > 197+Connect+REST+API+should+include+the+connector+type+
> > > > when+describing+a+connector
> > > >
> > > > Thanks
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-197: Include Connector type in Connector REST API
Posted by Randall Hauch <rh...@gmail.com>.
Hi, Ted. Thanks for the quick turn around.
I didn't remember that the response to the "/connectors/{name}/config" is
the actual configuration, so we probably shouldn't add "type" there. Sorry
about that.
And in the "/connector/{name}/status" response, I wonder if it would be
better to embed the "type" within the "connector" field, which corresponds
to adding the type to the `ConnectorStateInfo` (just like you did to the
`ConnectorInfo` class). WDYT?
Best regards,
Randall
On Fri, Sep 8, 2017 at 1:12 PM, Ted Yu <yu...@gmail.com> wrote:
> Thanks for the reminder, Randall.
>
> I have modified the KIP to include these two endpoints.
>
> On Fri, Sep 8, 2017 at 11:00 AM, Randall Hauch <rh...@gmail.com> wrote:
>
> > Hi, Ted.
> >
> > Thanks for creating this KIP and for working on the implementation. The
> > proposal looks great for the "/connectors/{name}" endpoint, but there are
> > several others that we need to consider as well so that the responses are
> > all consistent. In particular, look at "/connectors/{name}/status" and
> > "/connectors/{name}/config" (at least) that include the connector
> > information.
> >
> > Best regards,
> >
> > Randall
> >
> > On Fri, Sep 8, 2017 at 12:00 PM, Ted Yu <yu...@gmail.com> wrote:
> >
> > > Hi,
> > > Please take a look at:
> > >
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > 197+Connect+REST+API+should+include+the+connector+type+
> > > when+describing+a+connector
> > >
> > > Thanks
> > >
> >
>
Re: [DISCUSS] KIP-197: Include Connector type in Connector REST API
Posted by Ted Yu <yu...@gmail.com>.
Thanks for the reminder, Randall.
I have modified the KIP to include these two endpoints.
On Fri, Sep 8, 2017 at 11:00 AM, Randall Hauch <rh...@gmail.com> wrote:
> Hi, Ted.
>
> Thanks for creating this KIP and for working on the implementation. The
> proposal looks great for the "/connectors/{name}" endpoint, but there are
> several others that we need to consider as well so that the responses are
> all consistent. In particular, look at "/connectors/{name}/status" and
> "/connectors/{name}/config" (at least) that include the connector
> information.
>
> Best regards,
>
> Randall
>
> On Fri, Sep 8, 2017 at 12:00 PM, Ted Yu <yu...@gmail.com> wrote:
>
> > Hi,
> > Please take a look at:
> >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 197+Connect+REST+API+should+include+the+connector+type+
> > when+describing+a+connector
> >
> > Thanks
> >
>
Re: [DISCUSS] KIP-197: Include Connector type in Connector REST API
Posted by Randall Hauch <rh...@gmail.com>.
Hi, Ted.
Thanks for creating this KIP and for working on the implementation. The
proposal looks great for the "/connectors/{name}" endpoint, but there are
several others that we need to consider as well so that the responses are
all consistent. In particular, look at "/connectors/{name}/status" and
"/connectors/{name}/config" (at least) that include the connector
information.
Best regards,
Randall
On Fri, Sep 8, 2017 at 12:00 PM, Ted Yu <yu...@gmail.com> wrote:
> Hi,
> Please take a look at:
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 197+Connect+REST+API+should+include+the+connector+type+
> when+describing+a+connector
>
> Thanks
>