[DISCUSS] Introducing Builder in FlinkKafkaProducer

[Александр]

Hi, devs. I recently joined the community and I'm trying to understand the
project module by module. I've noticed that in FlinkKafkaProducer (
*https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-kafka/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaProducer.java
<https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-kafka/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaProducer.java>*
)
we have about ~ 8 constructors. Maybe we can use builder pattern here? It
seems the better solution. However, we have to keep our constructors for
backward compatibility (but mark it deprecated I think).

What do you think about this idea? I would like to focus on it.

BR,
Aleksandr Salatich

[Piotr Nowojski-3]

Hi,

This would definitely be beneficial. And yes, we would have to keep the current constructors for at least one release marked as @Deprecated.

Piotrek

> On 4 Mar 2019, at 08:49, Александр <[hidden email]> wrote:
>
> Hi, devs. I recently joined the community and I'm trying to understand the
> project module by module. I've noticed that in FlinkKafkaProducer (
> *https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-kafka/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaProducer.java
> <https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-kafka/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaProducer.java>*
> )
> we have about ~ 8 constructors. Maybe we can use builder pattern here? It
> seems the better solution. However, we have to keep our constructors for
> backward compatibility (but mark it deprecated I think).
>
> What do you think about this idea? I would like to focus on it.
>
> BR,
> Aleksandr Salatich

[Aljoscha Krettek-2]

I think before doing anything quick here we should look at this more holistically: How do the different connectors work, i.e. how do you construct them? Can we find a way to unify that, maybe using a Builder pattern? And then should we make a plan of getting the connectors there, possibly with a FLIP, because that sounds like a somewhat bigger change.

What do you think?

[1] https://cwiki.apache.org/confluence/display/FLINK/Flink+Improvement+Proposals

> On 4. Mar 2019, at 11:33, Piotr Nowojski <[hidden email]> wrote:
>
> Hi,
>
> This would definitely be beneficial. And yes, we would have to keep the current constructors for at least one release marked as @Deprecated.
>
> Piotrek
>
>> On 4 Mar 2019, at 08:49, Александр <[hidden email]> wrote:
>>
>> Hi, devs. I recently joined the community and I'm trying to understand the
>> project module by module. I've noticed that in FlinkKafkaProducer (
>> *https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-kafka/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaProducer.java
>> <https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-kafka/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaProducer.java>*
>> )
>> we have about ~ 8 constructors. Maybe we can use builder pattern here? It
>> seems the better solution. However, we have to keep our constructors for
>> backward compatibility (but mark it deprecated I think).
>>
>> What do you think about this idea? I would like to focus on it.
>>
>> BR,
>> Aleksandr Salatich
>

[Piotr Nowojski-3]

Definitely we could go one step further. Having one common way to initialise connectors (I think this definitely would need a Builder pattern) might be even better, but it’s also much larger change with more things that need to be considered. Like what about type safety of configuration parameters? (With unified construction it might be difficult/impossible). I’m not sure how realistic is to hope for this to happen soon? Who would drive the effort there?

Introducing a custom builder for FlinkKafkaProducer would be already an improvement (I was tempted to do this couple of times) over what we have now, even if it would be replaced/supplemented by a unified builder later on. The current chain of constructors is very ugly and hard to use. So I wouldn’t block this smaller improvement if we won’t be able to find someone who would be willing to work on a unified approach.

Piotrek

> On 4 Mar 2019, at 16:34, Aljoscha Krettek <[hidden email]> wrote:
>
> I think before doing anything quick here we should look at this more holistically: How do the different connectors work, i.e. how do you construct them? Can we find a way to unify that, maybe using a Builder pattern? And then should we make a plan of getting the connectors there, possibly with a FLIP, because that sounds like a somewhat bigger change.
>
> What do you think?
>
> [1] https://cwiki.apache.org/confluence/display/FLINK/Flink+Improvement+Proposals
>
>> On 4. Mar 2019, at 11:33, Piotr Nowojski <[hidden email]> wrote:
>>
>> Hi,
>>
>> This would definitely be beneficial. And yes, we would have to keep the current constructors for at least one release marked as @Deprecated.
>>
>> Piotrek
>>
>>> On 4 Mar 2019, at 08:49, Александр <[hidden email]> wrote:
>>>
>>> Hi, devs. I recently joined the community and I'm trying to understand the
>>> project module by module. I've noticed that in FlinkKafkaProducer (
>>> *https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-kafka/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaProducer.java
>>> <https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-kafka/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaProducer.java>*
>>> )
>>> we have about ~ 8 constructors. Maybe we can use builder pattern here? It
>>> seems the better solution. However, we have to keep our constructors for
>>> backward compatibility (but mark it deprecated I think).
>>>
>>> What do you think about this idea? I would like to focus on it.
>>>
>>> BR,
>>> Aleksandr Salatich
>>
>