To avoid a separate KIP, maybe we can agree on the grace period as part of this KIP. Maybe 3 releases (~1 year) is a good target?
Ismael On Thu, Apr 1, 2021, 6:39 PM Chia-Ping Tsai <chia7...@apache.org> wrote: > > 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 > > > > > >> >> > >> > > > > > > > > >> >> > >> > > > > > > > > >> >> > >> > > > > > > > >> >> > >> > > > > > > >> >> > >> > > > > > >> >> > > > > > > > >> >> > > > > > > >> >> > > > > > >> > > > > > > >> > > > > > > > > > > > > > > > > > > > > >
