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 <yuzhih...@gmail.com> wrote:
> Updated KIP-197 with the reference to KIP-151
>
> On Mon, Sep 11, 2017 at 8:24 PM, Ewen Cheslack-Postava <e...@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 <rha...@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 <yuzhih...@gmail.com> wrote:
> > >
> > > > Lowercase constants are generated by the enum.
> > > >
> > > > Updated KIP again.
> > > >
> > > > On Fri, Sep 8, 2017 at 4:08 PM, Randall Hauch <rha...@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 <yuzhih...@gmail.com>
> wrote:
> > > > >
> > > > > > Updated the KIP accordingly.
> > > > > >
> > > > > > Cheers
> > > > > >
> > > > > > On Fri, Sep 8, 2017 at 3:57 PM, Randall Hauch <rha...@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 <yuzhih...@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 <
> > rha...@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 <
> yuzhih...@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
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
