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