> Deprecating `send` is going to be extremely disruptive to all existing > users (if you use -Werror, it will require updating every call site). Have > we considered encouraging the usage of the new method while not deprecating > the old methods? We could consider deprecation down the line. The existing > methods work fine for many people, it doesn't seem like a good idea to > penalize them. > > Instead, we can make the new method available for people who benefit from > it. After a grace period (3 releases), we can consider deprecating. > Thoughts?
Fair enough. will remove deprecation. On 2021/03/31 14:41:22, Ismael Juma <ism...@juma.me.uk> wrote: > Hi Chia-Ping, > > Deprecating `send` is going to be extremely disruptive to all existing > users (if you use -Werror, it will require updating every call site). Have > we considered encouraging the usage of the new method while not deprecating > the old methods? We could consider deprecation down the line. The existing > methods work fine for many people, it doesn't seem like a good idea to > penalize them. > > Instead, we can make the new method available for people who benefit from > it. After a grace period (3 releases), we can consider deprecating. > Thoughts? > > Ismael > > On Tue, Mar 30, 2021 at 8:50 PM Chia-Ping Tsai <chia7...@apache.org> wrote: > > > hi, > > > > I have updated KIP according to my latest response. I will start vote > > thread next week if there is no more comments :) > > > > Best Regards, > > Chia-Ping > > > > On 2021/01/31 05:39:17, Chia-Ping Tsai <chia7...@apache.org> wrote: > > > It seems to me changing the input type might make complicate the > > migration from deprecated send method to new API. > > > > > > Personally, I prefer to introduce a interface called “SendRecord” to > > replace ProducerRecord. Hence, the new API/classes is shown below. > > > > > > 1. CompletionStage send(SendRecord) > > > 2. class ProducerRecord implement SendRecord > > > 3. Introduce builder pattern for SendRecord > > > > > > That includes following benefit. > > > > > > 1. Kafka users who don’t use both return type and callback do not need > > to change code even though we remove deprecated send methods. (of course, > > they still need to compile code with new Kafka) > > > > > > 2. Kafka users who need Future can easily migrate to new API by regex > > replacement. (cast ProduceRecord to SendCast and add toCompletableFuture) > > > > > > 3. It is easy to support topic id in the future. We can add new method > > to SendRecord builder. For example: > > > > > > Builder topicName(String) > > > Builder topicId(UUID) > > > > > > 4. builder pattern can make code more readable. Especially, Produce > > record has a lot of fields which can be defined by users. > > > — > > > Chia-Ping > > > > > > On 2021/01/30 22:50:36 Ismael Juma wrote: > > > > Another thing to think about: the consumer api currently has > > > > `subscribe(String|Pattern)` and a number of methods that accept > > > > `TopicPartition`. A similar approach could be used for the Consumer to > > work > > > > with topic ids or topic names. The consumer side also has to support > > > > regexes so it probably makes sense to have a separate interface. > > > > > > > > Ismael > > > > > > > > On Sat, Jan 30, 2021 at 2:40 PM Ismael Juma <ism...@juma.me.uk> wrote: > > > > > > > > > I think this is a promising idea. I'd personally avoid the overload > > and > > > > > simply have a `Topic` type that implements `SendTarget`. It's a mix > > of both > > > > > proposals: strongly typed, no overloads and general class names that > > > > > implement `SendTarget`. > > > > > > > > > > Ismael > > > > > > > > > > On Sat, Jan 30, 2021 at 2:22 PM Jason Gustafson <ja...@confluent.io> > > > > > wrote: > > > > > > > > > >> Giving this a little more thought, I imagine sending to a topic is > > the > > > > >> most > > > > >> common case, so maybe it's an overload worth having. Also, if > > `SendTarget` > > > > >> is just a marker interface, we could let `TopicPartition` implement > > it > > > > >> directly. Then we have: > > > > >> > > > > >> interface SendTarget; > > > > >> class TopicPartition implements SendTarget; > > > > >> > > > > >> CompletionStage<RecordMetadata> send(String topic, Record record); > > > > >> CompletionStage<RecordMetadata> send(SendTarget target, Record > > record); > > > > >> > > > > >> The `SendTarget` would give us a lot of flexibility in the future. > > It > > > > >> would > > > > >> give us a couple options for topic ids. We could either have an > > overload > > > > >> of > > > > >> `send` which accepts `Uuid`, or we could add a `TopicId` type which > > > > >> implements `SendTarget`. > > > > >> > > > > >> -Jason > > > > >> > > > > >> > > > > >> On Sat, Jan 30, 2021 at 1:11 PM Jason Gustafson <ja...@confluent.io > > > > > > > >> wrote: > > > > >> > > > > >> > Yeah, good question. I guess we always tend to regret using > > lower-level > > > > >> > types in these APIs. Perhaps there should be some kind of > > interface: > > > > >> > > > > > >> > interface SendTarget > > > > >> > class TopicIdTarget implements SendTarget > > > > >> > class TopicTarget implements SendTarget > > > > >> > class TopicPartitionTarget implements SendTarget > > > > >> > > > > > >> > Then we just have: > > > > >> > > > > > >> > CompletionStage<RecordMetadata> send(SendTarget target, Record > > record); > > > > >> > > > > > >> > Not sure if we could reuse `Record` in the consumer though. We do > > have > > > > >> > some state in `ConsumerRecord` which is not present in > > `ProducerRecord` > > > > >> > (e.g. offset). Perhaps we could provide a `Record` view from > > > > >> > `ConsumerRecord` for convenience. That would be useful for use > > cases > > > > >> which > > > > >> > involve reading from one topic and writing to another. > > > > >> > > > > > >> > -Jason > > > > >> > > > > > >> > On Sat, Jan 30, 2021 at 12:29 PM Ismael Juma <ism...@juma.me.uk> > > wrote: > > > > >> > > > > > >> >> Interesting idea. A couple of things to consider: > > > > >> >> > > > > >> >> 1. Would we introduce the Message concept to the Consumer too? I > > think > > > > >> >> that's what .NET does. > > > > >> >> 2. If we eventually allow a send to a topic id instead of topic > > name, > > > > >> >> would > > > > >> >> that result in two additional overloads? > > > > >> >> > > > > >> >> Ismael > > > > >> >> > > > > >> >> On Sat, Jan 30, 2021 at 11:38 AM Jason Gustafson < > > ja...@confluent.io> > > > > >> >> wrote: > > > > >> >> > > > > >> >> > For the sake of having another option to shoot down, we could > > take a > > > > >> >> page > > > > >> >> > from the .net client and separate the message data from the > > > > >> destination > > > > >> >> > (i.e. topic or partition). This would get around the need to > > use a > > > > >> new > > > > >> >> > verb. For example: > > > > >> >> > > > > > >> >> > CompletionStage<RecordMetadata> send(String topic, Message > > message); > > > > >> >> > CompletionStage<RecordMetadata> send(TopicPartition > > topicPartition, > > > > >> >> Message > > > > >> >> > message); > > > > >> >> > > > > > >> >> > -Jason > > > > >> >> > > > > > >> >> > > > > > >> >> > > > > > >> >> > On Sat, Jan 30, 2021 at 11:30 AM Jason Gustafson < > > ja...@confluent.io > > > > >> > > > > > >> >> > wrote: > > > > >> >> > > > > > >> >> > > I think this still makes sense as a separate KIP. For > > KIP-691, we > > > > >> are > > > > >> >> > just > > > > >> >> > > looking to help define the error contract for the new API. > > > > >> >> > > > > > > >> >> > > -Jason > > > > >> >> > > > > > > >> >> > > On Sat, Jan 30, 2021 at 8:39 AM Ismael Juma < > > ism...@juma.me.uk> > > > > >> >> wrote: > > > > >> >> > > > > > > >> >> > >> Are we saying that we won't pursue this KIP in favor of the > > other > > > > >> >> one? > > > > >> >> > >> > > > > >> >> > >> Ismael > > > > >> >> > >> > > > > >> >> > >> On Sat, Jan 30, 2021, 4:15 AM Chia-Ping Tsai < > > chia7...@apache.org > > > > >> > > > > > >> >> > wrote: > > > > >> >> > >> > > > > >> >> > >> > hi Jason > > > > >> >> > >> > > > > > >> >> > >> > Thanks for your response. "transmit" is good to me. > > > > >> >> > >> > > > > > >> >> > >> > As we discussed by email, KIP-706 is going to be merged to > > > > >> KIP-691( > > > > >> >> > >> > https://cwiki.apache.org/confluence/x/PSfZCQ). Hence, > > please > > > > >> feel > > > > >> >> > free > > > > >> >> > >> to > > > > >> >> > >> > replace "produce" by "transmit" in KIP-691. > > > > >> >> > >> > > > > > >> >> > >> > Best, > > > > >> >> > >> > Chia-Ping > > > > >> >> > >> > > > > > >> >> > >> > On 2021/01/30 00:48:38, Jason Gustafson < > > ja...@confluent.io> > > > > >> >> wrote: > > > > >> >> > >> > > Hi Chia-Ping, > > > > >> >> > >> > > > > > > >> >> > >> > > I think this is a great idea. It is a pity that we > > cannot > > > > >> >> continue > > > > >> >> > to > > > > >> >> > >> use > > > > >> >> > >> > > the `send` verb, but I don't see how we can. I know we > > > > >> considered > > > > >> >> > >> > > `transmit` as another option which is closer to `send`. > > That > > > > >> >> would > > > > >> >> > >> avoid > > > > >> >> > >> > > the redundancy when people choose the common "producer" > > > > >> variable > > > > >> >> > name. > > > > >> >> > >> > > > > > > >> >> > >> > > producer.transmit > > > > >> >> > >> > > > > > > >> >> > >> > > instead of > > > > >> >> > >> > > > > > > >> >> > >> > > producer.produce > > > > >> >> > >> > > > > > > >> >> > >> > > A couple alternatives might be `write` or `append`. I'm > > happy > > > > >> >> with > > > > >> >> > >> > > `produce` as well, but curious if others have thoughts. > > > > >> >> > >> > > > > > > >> >> > >> > > -Jason > > > > >> >> > >> > > > > > > >> >> > >> > > > > > > >> >> > >> > > On Wed, Jan 20, 2021 at 9:37 AM Chia-Ping Tsai < > > > > >> >> chia7...@apache.org > > > > >> >> > > > > > > >> >> > >> > wrote: > > > > >> >> > >> > > > > > > >> >> > >> > > > Dear all, > > > > >> >> > >> > > > > > > > >> >> > >> > > > I'd like to start the discussion thread for KIP-706: > > > > >> >> > >> > > > > > > > >> >> > >> > > > > > >> >> > >> > > > > >> >> > > > > > >> >> > > > > >> > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=100829459 > > > > >> >> > >> > > > > > > > >> >> > >> > > > KIP-706 is proposing to introduce new API > > "CompletionStage > > > > >> >> > >> > > > produce(record)" to Producer. Kafka users can leverage > > > > >> >> > >> CompletionStage > > > > >> >> > >> > to > > > > >> >> > >> > > > write asynchronous non-blocking code. CompletionStage > > is > > > > >> more > > > > >> >> > >> powerful > > > > >> >> > >> > than > > > > >> >> > >> > > > Future and callback. Also, the code using Future and > > > > >> callback > > > > >> >> can > > > > >> >> > be > > > > >> >> > >> > easily > > > > >> >> > >> > > > re-written by CompletionStage. > > > > >> >> > >> > > > > > > > >> >> > >> > > > Cheers, > > > > >> >> > >> > > > Chia-Ping > > > > >> >> > >> > > > > > > > >> >> > >> > > > > > > > >> >> > >> > > > > > > >> >> > >> > > > > > >> >> > >> > > > > >> >> > > > > > > >> >> > > > > > >> >> > > > > >> > > > > > >> > > > > > > > > > > > > > > >
