Hi Jason,
Thanks a lot for your feedback. The health interface does provide the same
information that's already exposed in the Connect Rest endpoints. Where it
would be useful is if we need to infer certain kind of health status based
on the connect and task status. One good example would be Livel
Hi Magesh,
Thanks for the KIP. It's definitely useful to have a pluggable auth layer,
as we have for the kafka brokers. I was a bit unclear why we needed to
expose all this health information in the context though. Since it is the
bigger part of the API, I was hoping to see a little more motivatio
Yup, thanks for the changes. The 'health' package in particular feels like
a nice fit given the way we expect it to be used.
-Ewen
On Wed, May 16, 2018 at 7:02 PM Randall Hauch wrote:
> Looks good to me. Thanks for quickly making the changes! Great work!
>
> Best regards,
>
> Randall
>
> > On M
Looks good to me. Thanks for quickly making the changes! Great work!
Best regards,
Randall
> On May 16, 2018, at 8:07 PM, Magesh Nandakumar wrote:
>
> Randall,
>
> I have adjusted the package names per Ewen's suggestions and also made some
> minor edits per your suggestions. Since there are
Randall,
I have adjusted the package names per Ewen's suggestions and also made some
minor edits per your suggestions. Since there are no major outstanding
issues, i'm moving this to vote.
Thanks
Magesh
On Wed, May 16, 2018 at 4:38 PM, Randall Hauch wrote:
> A few very minor suggestions:
>
>
>
Ewen,
Thanks for your comments. I have made the changes to the package names and
also moved the nested class up in the package.
Public API would include
*org.apache.kafka.connect.rest*
-ConnectClusterState
-ConnectRestExtension
-ConnectRestExtensionContext
*org.apache.kafka.connect.health*
A
A few very minor suggestions:
1. There are a few formatting issues with paragraphs that use a
monospace font. Minor, but it would be nice to fix.
2. Would be nice to link to the PR
3. Do we need the org.apache.kafka.connect.rest.extension.entities
package? Could we just move the tw
Hey,
Sorry for the late follow up. I just had a couple of minor questions about
details:
* Some of the public API being added is under a runtime package. But that
would be new for public API -- currently only things under the runtime
package use that package name. I think changing the package nam
Randall- I think I have addressed all the comments. Let me know if we can
take this to Vote.
Thanks
Magesh
On Tue, May 8, 2018 at 10:12 PM, Magesh Nandakumar
wrote:
> Hi All,
>
> I have updated the KIP to reflect changes based on the PR
> https://github.com/apache/kafka/pull/4931. Its mostly ha
Hi All,
I have updated the KIP to reflect changes based on the PR
https://github.com/apache/kafka/pull/4931. Its mostly has minor changes to
the interfaces and includes details on packages for the interfaces and the
classes. Let me know your thoughts.
Thanks
Magesh
On Fri, Apr 27, 2018 at 12:03
Great work, Magesh. I like the overall approach a lot, so I left some
pretty nuanced comments about specific details.
Best regards,
Randall
On Wed, Apr 25, 2018 at 3:03 PM, Magesh Nandakumar
wrote:
> Thanks Randall for your thoughts. I have created a replica of the required
> entities in the d
Thanks Randall for your thoughts. I have created a replica of the required
entities in the draft implementation. If you can take a look at the PR and
let me know your thoughts, I will update the KIP to reflect the same
https://github.com/apache/kafka/pull/4931
On Tue, Apr 24, 2018 at 11:44 AM, Ra
Magesh, I think our last emails cross in mid-stream.
We definitely want to put the new public interfaces/classes in the API
module, and implementation in the runtime module. Yes, this will affect the
design, since for example we don't want to expose runtime types to the API,
and we want to prevent
Thanks for updating the KIP, Magesh. You've resolved all of my concerns,
though I have one more: we should specify the package names for all new
interfaces/classes.
I'm looking forward to more feedback from others.
Best regards,
Randall
On Fri, Apr 20, 2018 at 12:17 AM, Magesh Nandakumar
wrote
Randall,
I'm trying to put together a PR for this KIP with the understanding that that
all public interfaces should live in the api module and in the process I
noticed the following
1. WorkerConfig is available in runtime. If we are exposing it to the
Extension, do we just move it to the A
Hi All,
I have updated the KIP with following changes
1. Expanded the Motivation section
2. Included details about the interface in the public interface section
3. Modified the config name to rest.extension.classes
4. Modified the ConnectRestExtension to include Configurable instead o
Hi Randall,
Thanks for your feedback. I also would like to go with
rest.extension.classes`.
For exposing Configurable, my original intention was just to expose that to
the extension because that's all one needs to register JAX RS resources.
The fact that we use Jersey shouldn't even be exposed in
On Thu, Apr 12, 2018 at 10:59 AM, Magesh Nandakumar
wrote:
> Hi Randall,
>
> Thanks a lot for your feedback.
>
> I will update the KIP to reflect your comments in (1), (2), (7) and (8).
>
Looking forward to these.
>
> For comment # (3) , while it would be great to automatically configure the
>
Hi Randall,
Thanks a lot for your feedback.
I will update the KIP to reflect your comments in (1), (2), (7) and (8).
For comment # (3) , while it would be great to automatically configure the
Rest Extensions, I would prefer that to be specified explicitly. Lets
assume a connector archive include
Very nice proposal, Magesh. I like the approach and the new concepts and
interfaces, but I do have a few comments/suggestions about some specific
details:
1. In the "Motivation" section, perhaps it makes sense to briefly
describe one or two somewhat concrete examples of how this is useful.
Sounds Good. I will update the KIP to include an extension that
authenticates Basic Auth Credentials against a properties file. Let me know
if that works.
Thanks
Magesh
On Wed, Apr 11, 2018 at 9:51 AM, Gwen Shapira wrote:
> Not a blocker, but it will continue a good tradition to add something
>
Not a blocker, but it will continue a good tradition to add something
simple that works. I'm thinking plain-text auth and maybe ZK storage (like
the Kafka authorizer). It doesn't have to provide real security in any
sense, but it will allow people to test and experiment. Otherwise, if I
write an im
Hi Gwen,
Thanks for your feedback. As part of this KIP, I was planning on adding a
skeleton implementation in the examples but that can be used only as a
reference. At this moment, there is no plan to add a concrete
implementation of the plugin. Let me know your thoughts :).
Thanks,
Magesh
On We
Thanks Magesh! The lack of authentication and authorization in the REST API
is definitely a painpoint and something that can prevent a company from
adopting connect (or surviving an audit after they did!).
One thing that isn't clear to me from the KIP: In the past when we
contributed pluggable int
Hi,
We have posted KIP-285: Connect Rest Extension Plugin to add the ability to
provide Rest Extensions to Connect Rest API.
https://cwiki.apache.org/confluence/display/KAFKA/KIP-285%3A+Connect+Rest+Extension+Plugin
Please take a look. Your feedback is appreciated.
Thanks,
Magesh
Hi,
We have posted KIP-285: Connect Rest Extension Plugin to add the ability to
provide Rest Extensions to Connect Rest API.
https://cwiki.apache.org/confluence/display/KAFKA/KIP-
285%3A+Connect+Rest+Extension+Plugin
Please take a look. Your feedback is appreciated.
Thanks,
Magesh
26 matches
Mail list logo