Hi,
regarding "breaking existing functionality" .. yes...that was me getting
confused about intended and existing functionality :)
You are right, this won't break anything that is currently working.
I'll leave placement of "name" parameter as is and open a new issue to
clarify this later on.
Kind regards,
Sönke
On Wed, Jul 5, 2017 at 5:41 AM, Gwen Shapira <g...@confluent.io> wrote:
> Hey,
>
> Nice research and summary.
>
> Regarding the ability to have a "nameless" connector - I'm pretty sure we
> never intended to allow that.
> I'm confused about breaking something that currently works though - since
> we get NPE, how will giving more intentional exceptions break anything?
>
> Regarding the placing of the name - inside or outside the config. It looks
> messy and I'm as confused as you are. I think Konstantine had some ideas
> how this should be resolved. I hope he responds, but I think that for your
> PR, just accept current mess as given...
>
> Gwen
>
> On Tue, Jul 4, 2017 at 3:28 AM Sönke Liebau
> <soenke.lie...@opencore.com.invalid> wrote:
>
> > While working on KAFKA-4930 and KAFKA-4938 I came across some sort of
> > fundamental questions about the rest api for creating connectors in Kafka
> > Connect that I'd like to put up for discussion.
> >
> > Currently requests that do not contain a "name" element on the top level
> > are not accepted by the API, but that is due to a NullPointerException
> [1]
> > so not entirely intentional. Previous (and current if the lines causing
> the
> > Exception are removed) functionality was to create a connector named
> "null"
> > if that parameter was missing. I am not sure if this is a good thing, as
> > for example that connector will be overwritten every time a new request
> > without a name is sent, as opposed to the expected warning that a
> connector
> > of that name already exists.
> >
> > I would propose to reject api calls without a name provided on the top
> > level, but this might break requests that currently work, so should
> > probably be mentioned in the release notes.
> >
> > ----
> >
> > Additionally, the "name" parameter is also copied into the "config"
> > sub-element of the connector request - unless a name parameter was
> provided
> > there in the original request[2].
> >
> > So this:
> >
> > {
> > "name": "connectorname",
> > "config": {
> > "connector.class":
> > "org.apache.kafka.connect.tools.MockSourceConnector",
> > "tasks.max": "1",
> > "topics": "test-topic"
> > }
> > }
> >
> > would become this:
> > {
> > "name": "connectorname",
> > "config": {
> > "name": "connectorname",
> > "connector.class":
> > "org.apache.kafka.connect.tools.MockSourceConnector",
> > "tasks.max": "1",
> > "topics": "test-topic"
> > }
> > }
> >
> > But a request that contains two different names like this:
> >
> > {
> > "name": "connectorname",
> > "config": {
> > "name": "differentconnectorname",
> > "connector.class":
> > "org.apache.kafka.connect.tools.MockSourceConnector",
> > "tasks.max": "1",
> > "topics": "test-topic"
> > }
> > }
> >
> > would be allowed as is.
> >
> > This might be intentional behavior in order to enable Connectors to have
> a
> > "name" parameter of their own - though I couldn't find any that do, but I
> > think this has the potential for misunderstandings, especially as there
> may
> > be code out there that references the connector name from the config
> object
> > and would thus grab the "wrong" one.
> >
> > Again, this may be intentional, so I am mostly looking for comments on
> how
> > to proceed here.
> >
> > My first instinct is to make the top-level "name" parameter mandatory as
> > suggested above and then add a check to reject requests that contain a
> > different "name" field in the config element.
> >
> > Any comments or thoughts welcome.
> >
> > TL/DR:
> > Two questions up for discussion:
> > 1. Should we reject api calls to create a connector that do not contain a
> > "name" element on the top level?
> > 2. Is there a use case where it makes sense to have different "name"
> > elements in the connector config and as the connector name?
> >
> > Kind regards,
> > Sönke
> >
> > [1] https://github.com/apache/kafka/blob/trunk/connect/
> > runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/
> > ConnectorsResource.java#L91
> >
> > [2] https://github.com/apache/kafka/blob/trunk/connect/
> > runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/
> > ConnectorsResource.java#L96
> >
>
--
Sönke Liebau
Partner
Tel. +49 179 7940878
OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel - Germany
