kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gwen Shapira <g...@confluent.io>
Subject Re: [DISCUSS] KAFKA-4930 & KAFKA 4938 - Treatment of name parameter in create connector requests
Date Wed, 05 Jul 2017 03:41:59 GMT
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.liebau@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
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message