Sounds good to me too.
As far as deprecating goes -- should the topics() method removed completely
or should it have a @deprecated annotation for removal in some future
version?
Best,
Nishanth Pradeep
On Sun, Jul 22, 2018 at 1:32 PM Matthias J. Sax <matth...@confluent.io>
wrote:
> Works for me.
>
> On 7/22/18 9:48 AM, Guozhang Wang wrote:
> > I think I can be convinced with deprecating topics() to keep API minimal.
> >
> > About renaming the others with `XXNames()`: well, to me it feels still
> not
> > very worthy since although it is not a big burden, it seems also not a
> big
> > "return" if we name the newly added function `topicSet()`.
> >
> >
> > Guozhang
> >
> >
> > On Fri, Jul 20, 2018 at 7:38 PM, Nishanth Pradeep <nishanth...@gmail.com
> >
> > wrote:
> >
> >> I definitely agree with you on deprecating topics().
> >>
> >> I also think changing the method names for consistency is reasonable,
> since
> >> there is no functionality change. Although, I can be convinced either
> way
> >> on this one.
> >>
> >> Best,
> >> Nishanth Pradeep
> >> On Fri, Jul 20, 2018 at 12:15 PM Matthias J. Sax <matth...@confluent.io
> >
> >> wrote:
> >>
> >>> I would still deprecate existing `topics()` method. If users need a
> >>> String, they can call `topicSet().toString()`.
> >>>
> >>> It's just a personal preference, because I believe it's good to keep
> the
> >>> API "minimal".
> >>>
> >>> About renaming the other methods: I thinks it's a very small burden to
> >>> deprecate the existing methods and add them with new names. Also just
> my
> >>> 2 cents.
> >>>
> >>> Would be good to see what others think.
> >>>
> >>>
> >>> -Matthias
> >>>
> >>> On 7/19/18 6:20 PM, Nishanth Pradeep wrote:
> >>>> Understood, Guozhang.
> >>>>
> >>>> Thanks for the help, everyone! I have updated the KIP. Let me know if
> >> you
> >>>> any other thoughts or suggestions.
> >>>>
> >>>> Best,
> >>>> Nishanth Pradeep
> >>>>
> >>>> On Thu, Jul 19, 2018 at 7:33 PM Guozhang Wang <wangg...@gmail.com>
> >>> wrote:
> >>>>
> >>>>> I see.
> >>>>>
> >>>>> Well, I think if we add a new function like topicSet() it is less
> >>> needed to
> >>>>> deprecate topics() as it returns "{topic1, topic2, ..}" which is sort
> >> of
> >>>>> non-overlapping in usage with the new API.
> >>>>>
> >>>>>
> >>>>> Guozhang
> >>>>>
> >>>>> On Thu, Jul 19, 2018 at 5:31 PM, Nishanth Pradeep <
> >>> nishanth...@gmail.com>
> >>>>> wrote:
> >>>>>
> >>>>>> That is what I meant. I will add topicSet() instead of changing the
> >>>>>> signature of topics() for compatibility reasons. But should we not
> >> add
> >>> a
> >>>>>> @deprecated flag for topics() or do you want to keep it around for
> >> the
> >>>>> long
> >>>>>> run?
> >>>>>>
> >>>>>> On Thu, Jul 19, 2018 at 7:27 PM Guozhang Wang <wangg...@gmail.com>
> >>>>> wrote:
> >>>>>>
> >>>>>>> We cannot change the signature of the function named "topics" from
> >>>>>> "String"
> >>>>>>> to "Set<String>", as Matthias mentioned it is a compatibility
> >> breaking
> >>>>>>> change.
> >>>>>>>
> >>>>>>> That's why I was proposing add a new function like "Set<String>
> >>>>>>> topicSet()", while keeping "String topics()" as is.
> >>>>>>>
> >>>>>>> Guozhang
> >>>>>>>
> >>>>>>> On Thu, Jul 19, 2018 at 5:22 PM, Nishanth Pradeep <
> >>>>> nishanth...@gmail.com
> >>>>>>>
> >>>>>>> wrote:
> >>>>>>>
> >>>>>>>> Right, adding topicNames() instead of changing the return type of
> >>>>>>> topics()
> >>>>>>>> in order preserve backwards compatibility is a good idea. But is
> it
> >>>>> not
> >>>>>>>> better to depreciate topics() because it would be redundant? In
> our
> >>>>>> case,
> >>>>>>>> it would only be calling topicNames/topicSet#toString().
> >>>>>>>>
> >>>>>>>> I still agree that perhaps changing the other API's might be
> >>>>>> unnecessary
> >>>>>>>> since it's only a name change.
> >>>>>>>>
> >>>>>>>> I have made the change to the KIP to only add, not change,
> >>>>> preexisting
> >>>>>>>> APIs. But where do we stand on deprecating topics()?
> >>>>>>>>
> >>>>>>>> Best,
> >>>>>>>> Nishanth Pradeep
> >>>>>>>>
> >>>>>>>> On Thu, Jul 19, 2018 at 1:44 PM Guozhang Wang <wangg...@gmail.com
> >
> >>>>>>> wrote:
> >>>>>>>>
> >>>>>>>>> Personally I'd prefer to keep the deprecation-related changes as
> >>>>>> small
> >>>>>>> as
> >>>>>>>>> possible unless they are really necessary, and hence I'd prefer
> to
> >>>>>> just
> >>>>>>>> add
> >>>>>>>>>
> >>>>>>>>> List<String> topicList() /* or Set<String> topicSet() */
> >>>>>>>>>
> >>>>>>>>> in addition to topicPattern to Source, in addition to
> >>>>>>>> `topicNameExtractor`
> >>>>>>>>> to Sink, and leaving the current APIs as-is.
> >>>>>>>>>
> >>>>>>>>> Guozhang
> >>>>>>>>>
> >>>>>>>>> On Thu, Jul 19, 2018 at 10:36 AM, Matthias J. Sax <
> >>>>>>> matth...@confluent.io
> >>>>>>>>>
> >>>>>>>>> wrote:
> >>>>>>>>>
> >>>>>>>>>> Thanks for updating the KIP.
> >>>>>>>>>>
> >>>>>>>>>> The current `Source` interface has a method `String topics()`
> >>>>> atm.
> >>>>>>>> Thus,
> >>>>>>>>>> we cannot just add `Set<String> Source#topics()` because this
> >>>>> would
> >>>>>>>>>> replace the existing method and would be an incompatible change.
> >>>>>>>>>>
> >>>>>>>>>> I think, we should deprecate `String topics()` and add a method
> >>>>>> with
> >>>>>>>>>> different name:
> >>>>>>>>>>
> >>>>>>>>>> `Set<String> Source#topicNames()`
> >>>>>>>>>>
> >>>>>>>>>> The method name `topicNames` is more appropriate anyway, as we
> >>>>>>> return a
> >>>>>>>>>> set of String (ie, names) but no `Topic` objects. This raises
> one
> >>>>>>> more
> >>>>>>>>>> thought: we might want to rename `Processor#stores()` to
> >>>>>>>>>> `Processor#storeNames()` as well ass `Sink#topic()` to
> >>>>>>>>>> `Sink#topicName()`, too. This would keep the naming in the API
> >>>>>>>>> consistent.
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> WDYT? Hope that other can chime in as well.
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> -Matthias
> >>>>>>>>>>
> >>>>>>>>>> On 7/18/18 7:49 PM, Nishanth Pradeep wrote:
> >>>>>>>>>>> I have revised the KIP
> >>>>>>>>>>> <https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >>>>>>>> 321%3A+Update+
> >>>>>>>>>> TopologyDescription+to+better+represent+Source+and+Sink+Nodes>.
> >>>>>>>>>>> Here is a summary of the changes.
> >>>>>>>>>>>
> >>>>>>>>>>> 1. Changed return type from String to Set<String> for
> >>>>>>>>> Source#topics().
> >>>>>>>>>>>
> >>>>>>>>>>> Set<String> Source#topics()
> >>>>>>>>>>>
> >>>>>>>>>>> 2. Added method in TopologyDescription#Source to return the
> >>>>>>>> Pattern
> >>>>>>>>>> used
> >>>>>>>>>>> by the Source node.
> >>>>>>>>>>>
> >>>>>>>>>>> Pattern Source#topicPattern()
> >>>>>>>>>>>
> >>>>>>>>>>> 3. Changed return type of Sink#topicNameExtractor from
> >>>>> Class<?
> >>>>>>>>> extends
> >>>>>>>>>>> TopicNameExtractor> to just TopicNameExtractor.
> >>>>>>>>>>>
> >>>>>>>>>>> TopicNameExtractor Sink#topicNameExtractor()
> >>>>>>>>>>>
> >>>>>>>>>>> Best,
> >>>>>>>>>>> Nishanth Pradeep
> >>>>>>>>>>>
> >>>>>>>>>>> On Sun, Jul 15, 2018 at 11:24 PM Guozhang Wang <
> >>>>>> wangg...@gmail.com
> >>>>>>>>
> >>>>>>>>>> wrote:
> >>>>>>>>>>>
> >>>>>>>>>>>> Hi Nishanth,
> >>>>>>>>>>>>
> >>>>>>>>>>>> Since even combining these two the scope is still relatively
> >>>>>> small
> >>>>>>>> I'd
> >>>>>>>>>>>> suggest just do it in one KIP if you're willing to work on
> >>>>> them.
> >>>>>>> If
> >>>>>>>>> you
> >>>>>>>>>> do
> >>>>>>>>>>>> not then pleas feel free to create the JIRA for the second
> >>>>> step
> >>>>>> so
> >>>>>>>>> that
> >>>>>>>>>>>> others can pick it up.
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> Guozhang
> >>>>>>>>>>>>
> >>>>>>>>>>>> On Sun, Jul 15, 2018 at 6:14 PM, Matthias J. Sax <
> >>>>>>>>> matth...@confluent.io
> >>>>>>>>>>>
> >>>>>>>>>>>> wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>>> There is no general protocol. We can include the changes in
> >>>>> the
> >>>>>>>>> current
> >>>>>>>>>>>>> KIP or do a second KIP.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> If you don't want to include the change in this KIP, please
> >>>>>>> create
> >>>>>>>> a
> >>>>>>>>>> new
> >>>>>>>>>>>>> JIRA to track the other changes. You can assign the JIRA to
> >>>>>>>> yourself
> >>>>>>>>>> and
> >>>>>>>>>>>>> start a second KIP if you want. You can also "link" both
> >>>>> JIRAs
> >>>>>> as
> >>>>>>>>>>>>> related to each other.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> -Matthias
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> On 7/15/18 12:50 PM, Nishanth Pradeep wrote:
> >>>>>>>>>>>>>> Thank you for the comments! I agree with these changes.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> So is the general protocol to update the same KIP, or is to
> >>>>>>> scrap
> >>>>>>>>> the
> >>>>>>>>>>>>>> current KIP and create a new one since it's beyond the scope
> >>>>>> of
> >>>>>>>> the
> >>>>>>>>>>>>>> original KIP? I am happy to do either.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> On Wed, Jul 4, 2018 at 1:48 PM Matthias J. Sax <
> >>>>>>>>> matth...@confluent.io
> >>>>>>>>>>>
> >>>>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Sounds good to me.
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> -Matthias
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> On 7/4/18 10:53 AM, Guozhang Wang wrote:
> >>>>>>>>>>>>>>>> After looked through the current TopologyDescription I
> >>>>> think
> >>>>>>> I'd
> >>>>>>>>>> want
> >>>>>>>>>>>>> to
> >>>>>>>>>>>>>>>> combine the suggestions from John and Matthias on the API
> >>>>>>>>> proposal.
> >>>>>>>>>>>> The
> >>>>>>>>>>>>>>>> motivations is that we have two relatively different
> >>>>>>>>> functionalities
> >>>>>>>>>>>>>>>> provided from the APIs today:
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> 1. Each interface's public functions, like
> >>>>>>>>>>>>>>>> SourceNode#topics(), GlobalStore#source(), which returns
> >>>>>>>>> non-String
> >>>>>>>>>>>>> typed
> >>>>>>>>>>>>>>>> data. The hope was to let users programmatically leverage
> >>>>> on
> >>>>>>>> those
> >>>>>>>>>>>> APIs
> >>>>>>>>>>>>>>> for
> >>>>>>>>>>>>>>>> runtime checking.
> >>>>>>>>>>>>>>>> 2. Each interface's impl class also have an implicit
> >>>>>>> toString()
> >>>>>>>>>>>>>>> overridden
> >>>>>>>>>>>>>>>> to print the necessary information. This was designed for
> >>>>>>>>> debugging
> >>>>>>>>>>>>>>>> purposes only during development cycles.
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> What we've observed so far, though, is that users leverage
> >>>>>> 2)
> >>>>>>>> much
> >>>>>>>>>>>> more
> >>>>>>>>>>>>>>>> than 1) in practice, since it is more convienent to parse
> >>>>>>>> strings
> >>>>>>>>>>>> than
> >>>>>>>>>>>>>>>> recursively calling the APIs to get non-string fields. On
> >>>>>> the
> >>>>>>>>> other
> >>>>>>>>>>>>> hand,
> >>>>>>>>>>>>>>>> the discussion controversy is more around 1), not 2). As
> >>>>> for
> >>>>>>> 2)
> >>>>>>>>>>>> people
> >>>>>>>>>>>>>>> seem
> >>>>>>>>>>>>>>>> to be on the right page anyways: print the topic lists if
> >>>>> it
> >>>>>>> is
> >>>>>>>>> not
> >>>>>>>>>>>>>>>> dynamic, or print extractor string format otherwise. For
> >>>>> 1)
> >>>>>>>> above
> >>>>>>>>> we
> >>>>>>>>>>>>>>> should
> >>>>>>>>>>>>>>>> probably have all three `Set<String> topics()`, `Pattern
> >>>>>>>>>>>>> topicPattern()`
> >>>>>>>>>>>>>>>> and `TopicNameExtractor topicExtractor()`; while for 2) I
> >>>>>> feel
> >>>>>>>>>>>>>>> comfortable
> >>>>>>>>>>>>>>>> relying on the TopicNameExtractor#toString() in
> >>>>>>>>> `Source#toString()`
> >>>>>>>>>>>>> impl
> >>>>>>>>>>>>>>>> since even if users do not override this function, the
> >>>>>> default
> >>>>>>>>> value
> >>>>>>>>>>>>>>>> `className@hashcode` still looks fine to me.
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> Guozhang
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> On Tue, Jul 3, 2018 at 11:22 PM, Matthias J. Sax <
> >>>>>>>>>>>>> matth...@confluent.io>
> >>>>>>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> I just double checked the discussion thread of KIP-120
> >>>>> that
> >>>>>>>>>>>> introduced
> >>>>>>>>>>>>>>>>> `TopologyDescription`. Back than the argument was, that
> >>>>>> using
> >>>>>>>> the
> >>>>>>>>>>>>>>>>> simplest option might be sufficient because the
> >>>>> description
> >>>>>>> is
> >>>>>>>>>>>> mostly
> >>>>>>>>>>>>>>>>> used for debugging.
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> Not sure if this argument holds. It seem that people
> >>>>> built
> >>>>>>>> first
> >>>>>>>>>>>> more
> >>>>>>>>>>>>>>>>> sophisticated tools using TopologyDescription.
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> Final note: if we really want to add `topicPattern()` we
> >>>>>>> might
> >>>>>>>>> want
> >>>>>>>>>>>> to
> >>>>>>>>>>>>>>>>> deprecate `topic()` and replace with `Set<String>
> >>>>>> topics()`,
> >>>>>>>>>>>> because a
> >>>>>>>>>>>>>>>>> source node can take a multiple topics, too.
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> Just adding this for completeness of context to the
> >>>>>>> discussion.
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> -Matthias
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> On 7/3/18 11:09 PM, Matthias J. Sax wrote:
> >>>>>>>>>>>>>>>>>> John,
> >>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>> I am a little bit on the fence. In retrospective, it
> >>>>> might
> >>>>>>>> have
> >>>>>>>>>>>> been
> >>>>>>>>>>>>>>>>>> better to add `topic()` and `topicPattern()` to source
> >>>>>> node
> >>>>>>>> and
> >>>>>>>>>>>>> return
> >>>>>>>>>>>>>>> a
> >>>>>>>>>>>>>>>>>> proper `Pattern` object instead of the pattern as a
> >>>>>> String.
> >>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>> All other "payload" is just names and thus String
> >>>>>> naturally.
> >>>>>>>>> From
> >>>>>>>>>>>> my
> >>>>>>>>>>>>>>>>>> point of view `TopologyDescription` should represent the
> >>>>>>>>>> `Topology`
> >>>>>>>>>>>>> in
> >>>>>>>>>>>>>>> a
> >>>>>>>>>>>>>>>>>> "machine readable" form plus a default "human readable"
> >>>>>> from
> >>>>>>>> via
> >>>>>>>>>>>>>>>>>> `toString()` -- this does not imply that all return
> >>>>> types
> >>>>>>>> should
> >>>>>>>>>> be
> >>>>>>>>>>>>>>>>> String.
> >>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>> Let me know what you think. If you agree, we could even
> >>>>>> add
> >>>>>>>>>>>>>>>>>> `Source#topicPattern()` in another KIP.
> >>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>> -Matthias
> >>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>> On 6/26/18 3:45 PM, John Roesler wrote:
> >>>>>>>>>>>>>>>>>>> Sorry for the late comment,
> >>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>> Looking at the other pieces of TopologyDescription, I
> >>>>>>> noticed
> >>>>>>>>>> that
> >>>>>>>>>>>>>>>>> pretty
> >>>>>>>>>>>>>>>>>>> much all of the "payload" of these description nodes
> >>>>> are
> >>>>>>>>> strings.
> >>>>>>>>>>>>>>>>> Should we
> >>>>>>>>>>>>>>>>>>> consider returning a string from `topicNameExtractor()`
> >>>>>>>>> instead?
> >>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>> In fact, if we did that, we could consider calling
> >>>>>>>> `toString()`
> >>>>>>>>>> on
> >>>>>>>>>>>>> the
> >>>>>>>>>>>>>>>>>>> extractor instead of returning the class name. This
> >>>>> would
> >>>>>>>> allow
> >>>>>>>>>>>>>>> authors
> >>>>>>>>>>>>>>>>> of
> >>>>>>>>>>>>>>>>>>> the extractors to provide more information about the
> >>>>>>>> extractor
> >>>>>>>>>>>> than
> >>>>>>>>>>>>>>> just
> >>>>>>>>>>>>>>>>>>> its name. This might be especially useful in the case
> >>>>> of
> >>>>>>>>>> anonymous
> >>>>>>>>>>>>>>>>>>> implementations.
> >>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>> Thanks for the KIP,
> >>>>>>>>>>>>>>>>>>> -John
> >>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>> On Mon, Jun 25, 2018 at 11:52 PM Ted Yu <
> >>>>>>> yuzhih...@gmail.com
> >>>>>>>>>
> >>>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>> My previous response was talking about the new method
> >>>>> in
> >>>>>>>>>>>>>>>>>>>> InternalTopologyBuilder.
> >>>>>>>>>>>>>>>>>>>> The exception just means there is no uniform extractor
> >>>>>> for
> >>>>>>>> all
> >>>>>>>>>>>> the
> >>>>>>>>>>>>>>>>> sinks.
> >>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>> On Mon, Jun 25, 2018 at 8:02 PM, Matthias J. Sax <
> >>>>>>>>>>>>>>>>> matth...@confluent.io>
> >>>>>>>>>>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>> Ted,
> >>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>> Why? Each sink can have a different
> >>>>> TopicNameExtractor.
> >>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>> -Matthias
> >>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>> On 6/25/18 5:19 PM, Ted Yu wrote:
> >>>>>>>>>>>>>>>>>>>>>> If there are different TopicNameExtractor classes
> >>>>> from
> >>>>>>>>>> multiple
> >>>>>>>>>>>>>>> sink
> >>>>>>>>>>>>>>>>>>>>> nodes,
> >>>>>>>>>>>>>>>>>>>>>> the new method should throw exception alerting user
> >>>>> of
> >>>>>>>> such
> >>>>>>>>>>>>>>> scenario.
> >>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>> On Mon, Jun 25, 2018 at 2:23 PM, Bill Bejeck <
> >>>>>>>>>>>> bbej...@gmail.com>
> >>>>>>>>>>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>> Thanks for the KIP!
> >>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>> Overall I'm +1 on the KIP. I have one question.
> >>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>> The KIP states that the method
> >>>>> "topicNameExtractor()"
> >>>>>>> is
> >>>>>>>>>> added
> >>>>>>>>>>>>> to
> >>>>>>>>>>>>>>>>> the
> >>>>>>>>>>>>>>>>>>>>>>> InternalTopologyBuilder.java.
> >>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>> It could be that I'm missing something, but wow
> >>>>> does
> >>>>>>> this
> >>>>>>>>>> work
> >>>>>>>>>>>>> if
> >>>>>>>>>>>>>>> a
> >>>>>>>>>>>>>>>>>>>> user
> >>>>>>>>>>>>>>>>>>>>>>> has provided different TopicNameExtractor instances
> >>>>>> to
> >>>>>>>>>>>> multiple
> >>>>>>>>>>>>>>> sink
> >>>>>>>>>>>>>>>>>>>>> nodes?
> >>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>> Thanks,
> >>>>>>>>>>>>>>>>>>>>>>> Bill
> >>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>> On Mon, Jun 25, 2018 at 1:25 PM Guozhang Wang <
> >>>>>>>>>>>>> wangg...@gmail.com
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>> Yup I agree, generally speaking the `toString()`
> >>>>>>> output
> >>>>>>>> is
> >>>>>>>>>>>> not
> >>>>>>>>>>>>>>>>>>>>>>> recommended
> >>>>>>>>>>>>>>>>>>>>>>>> to be relied on programmatically in user's code,
> >>>>> but
> >>>>>>>> we've
> >>>>>>>>>>>>>>> observed
> >>>>>>>>>>>>>>>>>>>>>>>> convenience-beats-any-other-reasons again and
> >>>>>> again in
> >>>>>>>>>>>>>>> development
> >>>>>>>>>>>>>>>>>>>>>>>> unfortunately. I think we should still not
> >>>>> claiming
> >>>>>> it
> >>>>>>>> is
> >>>>>>>>>>>> part
> >>>>>>>>>>>>> of
> >>>>>>>>>>>>>>>>> the
> >>>>>>>>>>>>>>>>>>>>>>>> public APIs that would not be changed anyhow in
> >>>>> the
> >>>>>>>>> future,
> >>>>>>>>>>>> but
> >>>>>>>>>>>>>>>>> just
> >>>>>>>>>>>>>>>>>>>>>>>> mentioning it in the wiki for people to be aware
> >>>>> is
> >>>>>>>> fine.
> >>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>> Guozhang
> >>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>> On Sun, Jun 24, 2018 at 5:01 PM, Matthias J. Sax <
> >>>>>>>>>>>>>>>>>>>>> matth...@confluent.io>
> >>>>>>>>>>>>>>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>> Thanks for the KIP!
> >>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>> I am don't have any further comments.
> >>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>> For Guozhang's comment: if we mention anything
> >>>>>> about
> >>>>>>>>>>>>>>> `toString()`,
> >>>>>>>>>>>>>>>>>>>> we
> >>>>>>>>>>>>>>>>>>>>>>>>> should make explicit that `toString()` output is
> >>>>>>> still
> >>>>>>>>> not
> >>>>>>>>>>>>>>> public
> >>>>>>>>>>>>>>>>>>>>>>>>> contract and users should not rely on the output.
> >>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>> Furhtermore, for the actual uses output, I would
> >>>>>>>> replace
> >>>>>>>>>>>>>>> "topic:"
> >>>>>>>>>>>>>>>>> by
> >>>>>>>>>>>>>>>>>>>>>>>>> "extractor class:" to make the difference
> >>>>> obvious.
> >>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>> I am just hoping that people actually to not rely
> >>>>>> on
> >>>>>>>>>>>>>>> `toString()`
> >>>>>>>>>>>>>>>>>>>> what
> >>>>>>>>>>>>>>>>>>>>>>>>> defeats the purpose to the `TopologyDescription`
> >>>>>>> class
> >>>>>>>>> that
> >>>>>>>>>>>>> was
> >>>>>>>>>>>>>>>>>>>>>>>>> introduced to avoid the dependency... (Just a
> >>>>> side
> >>>>>>>>> comment,
> >>>>>>>>>>>>> not
> >>>>>>>>>>>>>>>>>>>> really
> >>>>>>>>>>>>>>>>>>>>>>>>> related to this KIP proposal itself).
> >>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>> If there are no further comments in the next
> >>>>> days,
> >>>>>>> feel
> >>>>>>>>>> free
> >>>>>>>>>>>>> to
> >>>>>>>>>>>>>>>>>>>> start
> >>>>>>>>>>>>>>>>>>>>>>>>> the VOTE and open a PR.
> >>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>> -Matthias
> >>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>> On 6/22/18 6:04 PM, Guozhang Wang wrote:
> >>>>>>>>>>>>>>>>>>>>>>>>>> Thanks for writing the KIP!
> >>>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>> I'm +1 on the proposed changes over all. One
> >>>>> minor
> >>>>>>>>>>>>> suggestion:
> >>>>>>>>>>>>>>> we
> >>>>>>>>>>>>>>>>>>>>>>>> should
> >>>>>>>>>>>>>>>>>>>>>>>>>> also mention that the `Sink#toString` will also
> >>>>> be
> >>>>>>>>>> updated,
> >>>>>>>>>>>>> in
> >>>>>>>>>>>>>>> a
> >>>>>>>>>>>>>>>>>>>> way
> >>>>>>>>>>>>>>>>>>>>>>>> that
> >>>>>>>>>>>>>>>>>>>>>>>>>> if `topic()` returns null, use the other call,
> >>>>>> etc.
> >>>>>>>> This
> >>>>>>>>>> is
> >>>>>>>>>>>>>>>>> because
> >>>>>>>>>>>>>>>>>>>>>>>>>> although we do not explicitly state the
> >>>>> following
> >>>>>>>> logic
> >>>>>>>>> as
> >>>>>>>>>>>>>>> public
> >>>>>>>>>>>>>>>>>>>>>>>>> protocols:
> >>>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>> ```
> >>>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>> "Sink: " + name + " (topic: " + topic() + ")\n
> >>>>>>>> <--
> >>>>>>>>> "
> >>>>>>>>>> +
> >>>>>>>>>>>>>>>>>>>>>>>>>> nodeNames(predecessors);
> >>>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>> ```
> >>>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>> There are already some users that rely on
> >>>>>>>>>>>>>>>>>>>>>>>> `topology.describe().toString(
> >>>>>>>>>>>>>>>>>>>>>>>>> )`
> >>>>>>>>>>>>>>>>>>>>>>>>>> to have runtime checks on the returned string
> >>>>>>> values,
> >>>>>>>> so
> >>>>>>>>>>>>>>> changing
> >>>>>>>>>>>>>>>>>>>>>>> this
> >>>>>>>>>>>>>>>>>>>>>>>>>> means that their app will break and hence need
> >>>>> to
> >>>>>>> make
> >>>>>>>>>> code
> >>>>>>>>>>>>>>>>>>>> changes.
> >>>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>> Guozhang
> >>>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Jun 20, 2018 at 7:20 PM, Nishanth
> >>>>> Pradeep
> >>>>>> <
> >>>>>>>>>>>>>>>>>>>>>>>> nishanth...@gmail.com
> >>>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>>> Hello Everyone,
> >>>>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>>> I have created a new KIP to discuss extending
> >>>>>>>>>>>>>>>>> TopologyDescription.
> >>>>>>>>>>>>>>>>>>>>>>> You
> >>>>>>>>>>>>>>>>>>>>>>>>> can
> >>>>>>>>>>>>>>>>>>>>>>>>>>> find it here:
> >>>>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>>> https://cwiki.apache.org/
> >>>>>>>> confluence/display/KAFKA/KIP-
> >>>>>>>>>>>>>>>>>>>>>>>>>>> 321%3A+Add+method+to+get+TopicNameExtractor+in+
> >>>>>>>>>>>>>>>>> TopologyDescription
> >>>>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>>> Please provide any feedback that you might
> >>>>> have.
> >>>>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>>> Best,
> >>>>>>>>>>>>>>>>>>>>>>>>>>> Nishanth Pradeep
> >>>>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>> --
> >>>>>>>>>>>>>>>>>>>>>>>> -- Guozhang
> >>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> --
> >>>>>>>>>>>> -- Guozhang
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> --
> >>>>>>>>> -- Guozhang
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> --
> >>>>>>> -- Guozhang
> >>>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> --
> >>>>> -- Guozhang
> >>>>>
> >>>>
> >>>
> >>>
> >>
> >
> >
> >
>
>
