kafka-connect-client icon indicating copy to clipboard operation
kafka-connect-client copied to clipboard

NewConnectorDefinition.config should be Map<String, Object>

Open MosheElisha opened this issue 4 years ago • 2 comments

Hi,

I believe NewConnectorDefinition.config should be Map<String, Object> instead of Map<String, String>. This will better support non-string configuration such as tasks.max and errors.deadletterqueue.topic.replication.factor which are numbers.

Perhaps for backward-compatibility there is a need to support both somehow.

MosheElisha avatar Jul 14 '21 14:07 MosheElisha

I can't remember if there's a reason why I explicitly made it Map<String, String> now, but this setter seems to accept numbers without a problem:

public Builder withConfig(final String key, final Object value) {
            this.config.put(key, value.toString());
            return this;
        }

And used in this test here: https://github.com/SourceLabOrg/kafka-connect-client/blob/master/src/test/java/org/sourcelab/kafka/connect/apiclient/KafkaConnectClientTest.java#L155-L163

Is there some other use case where this is problematic?

Crim avatar Jul 21 '21 03:07 Crim

It is not a blocker and when I send string values to numeric conf properties ("tasks.max"="1") it works fine.

The problem was that I already had a Map<String, Object> at hand and I wanted to simply pass that to the NewConnectorDefinition

MosheElisha avatar Jul 21 '21 08:07 MosheElisha