Agreed, I was proposing those as two possible options we can consider. If we add the store type enum first, then we could leverage that for this; if not, (which seems most likely), then we should just use the metricsScope which should be just as good (although not identical)
We can always revisit this part of the API if/when we tackle KIP-591 and consider migrating the topology description to using the new store enum. On Mon, Oct 5, 2020 at 3:45 PM Guozhang Wang <wangg...@gmail.com> wrote: > That's a good idea, I think StoreBuilder#metricsScope() is not a very > intrusive API to add. But note that the metricsScope() is not identical to > KIP-591's store type enum, e.g. the former's possible values include > "rocksdb-session" and "rocksdb-window". > > > Guozhang > > On Mon, Oct 5, 2020 at 2:21 PM Sophie Blee-Goldman <sop...@confluent.io> > wrote: > > > I suppose we could add a method to the StoreBuilder interface that calls > > through > > to the metricsScope() method of the StoreSupplier, similar to what we do > > for the store > > name. > > > > It feels a bit indirect but the metricsScope() should be an accurate > > description of > > the underlying store type. The whole point of metricsScope() is to > identify > > the store > > type for use in metrics, so it seems like a reasonable extension to use > it > > to identify > > the store type in the topology description as well. > > > > Or, if KIP-591 ever gets resurrected, maybe we will have a new store type > > enum or > > other public API to identify the stores that we can leverage here. But > that > > KIP seems > > to have gone dormant as well :) > > > > On Fri, Oct 2, 2020 at 6:18 PM Guozhang Wang <wangg...@gmail.com> wrote: > > > > > Hey Sophie, > > > > > > I've thought about this as well. But the tricky thing is that the > > topology > > > description's state store input is from the `StoreBuilder` class, which > > > does not include type information. If we want to peek into such info we > > > could call its `build` function, get the actual store and dig it out, > but > > > this would build the actual store even before the tasks are assigned > etc. > > > > > > We can, however, extend the API of StoreBuilder to expose its store > type > > > information but we need to be careful here: the interface is a public > API > > > and information too specific like `RocksDBWindow` may be leaking too > much > > > here. WDYT? > > > > > > > > > Guozhang > > > > > > On Tue, Sep 29, 2020 at 8:12 PM Sophie Blee-Goldman < > sop...@confluent.io > > > > > > wrote: > > > > > > > Hey Guozhang, what's the status of this KIP? > > > > > > > > I was recently digging through a particularly opaque Streams > > application > > > > and > > > > it occurred to me that it might also be useful to print the kind of > > store > > > > attached > > > > to each node (eg RocksDBWindowStore, InMemoryKeyValueStore, custom, > > > > etc). That made me think of this KIP so I was just wondering where it > > > ended > > > > up. And if you want to pick it up again, WDYT about including some > > minor > > > > store information in the augmented description? > > > > > > > > On Tue, May 19, 2020 at 1:22 PM Guozhang Wang <wangg...@gmail.com> > > > wrote: > > > > > > > > > We already has a Serdes actually, which is a factory class. What we > > > > really > > > > > need is to add new functions to `Serde`, `Serializer` and > > > `Deserializer` > > > > > interfaces, but since we already dropped Java7 backward > compatibility > > > may > > > > > not be a big issue anyways, let me think about it a bit more. > > > > > > > > > > On Tue, May 19, 2020 at 12:01 PM Matthias J. Sax <mj...@apache.org > > > > > > wrote: > > > > > > > > > > > Thanks Guozhang. > > > > > > > > > > > > This makes sense. I am still wondering about wrapped serdes: > > > > > > > > > > > > > and if it is a wrapper serde, also print its inner > > > > > > >>> serde name > > > > > > > > > > > > How can our default implementation of `TopologyDescriber` know if > > > it's > > > > a > > > > > > wrapped serde or not? Furthermore, how do wrapped serdes expose > > their > > > > > > inner serdes? > > > > > > > > > > > > I am also not sure what the purpose of TopologyDescriber is? > Would > > it > > > > > > mabye be better to add new interface `Serdes` can implement > > instead? > > > > > > > > > > > > > > > > > > -Matthias > > > > > > > > > > > > > > > > > > > > > > > > On 5/18/20 9:24 PM, Guozhang Wang wrote: > > > > > > > Bruno, Matthias: > > > > > > > > > > > > > > Thanks for your inputs. After some thoughts I've decide to > update > > > my > > > > > > > proposal in the following way: > > > > > > > > > > > > > > 1. Store#serdes() would return a "Map<String, String>" > > > > > > > > > > > > > > 2. Topology's description would be independent of whether it is > > > > > generated > > > > > > > from `StreamsBuilder#build(props)` or `StreamsBuilder#build()`, > > and > > > > if > > > > > > the > > > > > > > serde is not known we would use "<unknown>" as the default > value. > > > > > > > > > > > > > > 3. Add `List<String> TopologyDescription#sourceTopics() / > > > > sinkTopics() > > > > > / > > > > > > > repartitionTopics() / changelogTopics()` and for pattern / > > > > > > topic-extractor > > > > > > > we would use fixed format of "<pattern:regex>" and > > > > > > > "<dynamic:extractor-class-name>". > > > > > > > > > > > > > > > > > > > > > I will try to implement this in my existing PR and after I've > > > > confirmed > > > > > > it > > > > > > > works, I will update the final wiki for voting. > > > > > > > > > > > > > > > > > > > > > Guozhang > > > > > > > > > > > > > > > > > > > > > On Mon, May 18, 2020 at 9:13 PM Guozhang Wang < > > wangg...@gmail.com> > > > > > > wrote: > > > > > > > > > > > > > >> Hello Andy, > > > > > > >> > > > > > > >> Thanks a lot for your comments! I do not mind at all :) > > > > > > >> > > > > > > >> I think that's a valid point, what I have in mind is to expose > > an > > > > > > >> interface which can be optionally overridden in the overridden > > > > > > describe() > > > > > > >> call: > > > > > > >> > > > > > > >> Topology#describe(final TopologyDescriber) > > > > > > >> > > > > > > >> Interface TopologyDescriber { > > > > > > >> > > > > > > >> default describeSerde(final Serde); > > > > > > >> > > > > > > >> default describeSerializer(final Serializer); > > > > > > >> > > > > > > >> default describeDeserializer(final Serializer); > > > > > > >> } > > > > > > >> > > > > > > >> And we would expose a DefaultTopologyDescriber class that just > > > print > > > > > the > > > > > > >> serde class names -- and if it is a wrapper serde, also print > > its > > > > > inner > > > > > > >> serde name. > > > > > > >> > > > > > > >> Guozhang > > > > > > >> > > > > > > >> > > > > > > >> On Mon, May 11, 2020 at 12:13 PM Andy Coates < > a...@confluent.io > > > > > > > > wrote: > > > > > > >> > > > > > > >>> Hi Guozhang, > > > > > > >>> > > > > > > >>> Thanks for writing this up. I’m very interested to see this, > > so I > > > > > hope > > > > > > >>> you don’t mind me commenting. > > > > > > >>> > > > > > > >>> I’ve only really one comment to make, and that’s on the text > > > > printed > > > > > > for > > > > > > >>> the serde classes: > > > > > > >>> > > > > > > >>> As I understand it, the name will either come from the passed > > in > > > > > > config, > > > > > > >>> or may default to “unknown”, or may be obtained from the > > > instances > > > > > > passed > > > > > > >>> while building the topology. It’s this latter case that > > interests > > > > me. > > > > > > >>> Where you have an actual serde instance could we not output > > more > > > > > > >>> information? > > > > > > >>> > > > > > > >>> The examples use simple (de)serialization classes such as > > > > > > >>> `LongDeseriailizer` where the name alone imparts all the > > > > information > > > > > > the > > > > > > >>> user is likely to need. However, users may provide there own > > > custom > > > > > > >>> serialisers and such serialisers may contain state that is > > > > important, > > > > > > e.g. > > > > > > >>> the serialiser may know the schema of the data being > > serialized. > > > > May > > > > > > there > > > > > > >>> be benefit from taking the `toString()` representation of the > > > > > > serialiser? > > > > > > >>> > > > > > > >>> Of course, this would require adding suitable `toString` > impls > > to > > > > our > > > > > > own > > > > > > >>> stock serialisers, but may ultimately prove more versatile in > > the > > > > > > future. > > > > > > >>> The downside is potential to corrupt the topology > description, > > > > e.g. a > > > > > > >>> toString that includes new lines etc. > > > > > > >>> > > > > > > >>> Thanks, > > > > > > >>> > > > > > > >>> Andy > > > > > > >>> > > > > > > >>> > > > > > > >>> > > > > > > >>>> On 4 May 2020, at 19:27, Bruno Cadonna <br...@confluent.io> > > > > wrote: > > > > > > >>>> > > > > > > >>>> Hi Guozhang, > > > > > > >>>> > > > > > > >>>> Thank you for the KIP! > > > > > > >>>> > > > > > > >>>> Exposing also the inner types of the wrapper serdes would be > > > > > > >>>> important. For debugging as Matthias has already mentioned > and > > > to > > > > > see > > > > > > >>>> more easily changes that are applied to a topology. > > > > > > >>>> > > > > > > >>>> I am also +1 on the `toJson()` method to easily access the > > > > topology > > > > > > >>>> description programmatically and to make the description > > > backward > > > > > > >>>> compatible. > > > > > > >>>> > > > > > > >>>> Regarding `List<String> serdeNames();`, I would be in favour > > of > > > a > > > > > more > > > > > > >>>> expressive return type, like a Map that assigns labels to > > Serde > > > > > names. > > > > > > >>>> For example, for key and value serdes the label could be > "key" > > > and > > > > > > >>>> "value". Or something similar. > > > > > > >>>> > > > > > > >>>> Best, > > > > > > >>>> Bruno > > > > > > >>>> > > > > > > >>>> > > > > > > >>>> > > > > > > >>>> On Mon, May 4, 2020 at 2:25 AM Guozhang Wang < > > > wangg...@gmail.com> > > > > > > >>> wrote: > > > > > > >>>>> > > > > > > >>>>> Hello Matthias John, thanks for your comments!! Replied > them > > > > > inline. > > > > > > >>>>> > > > > > > >>>>> I think there are a couple open questions that I'd like to > > hear > > > > > your > > > > > > >>>>> opinions on with the context: > > > > > > >>>>> > > > > > > >>>>> a. For stores's serdes, the reason I proposed to expose a > set > > > of > > > > > > serde > > > > > > >>>>> names instead of a pair of key / value serdes is for future > > > > > possible > > > > > > >>> store > > > > > > >>>>> types which may not be key-values. I admit it could just be > > > > > > >>> over-killing > > > > > > >>>>> here so if you have a strong preference on the latter, I > > could > > > be > > > > > > >>> convinced > > > > > > >>>>> to change that part but I'd want to make the original > > > motivation > > > > > > clear. > > > > > > >>>>> > > > > > > >>>>> b. I think I'm convinced that I'd just augment the > `toString` > > > > > result > > > > > > >>>>> regardless of which func generated the Topology (and hence > > its > > > > > > >>>>> TopologyDescription), note this would mean that we break > the > > > > > > >>> compatibility > > > > > > >>>>> of the current `toString` function. As a remedy for that, > we > > > will > > > > > > also > > > > > > >>>>> expose a `toJson` function to programmatical purposes. > > > > > > >>>>> > > > > > > >>>>> Guozhang > > > > > > >>>>> > > > > > > >>>>> > > > > > > >>>>>> (1) In the new TopologyDescription output, the line for > the > > > > > > >>>>>> windowed-count processor is: > > > > > > >>>>>> > > > > > > >>>>>>> Processor: myname (stores: [(myname-store, serdes: > > > > > > >>>>> [SessionWindowedSerde, FullChangeSerde])]) > > > > > > >>>>>> > > > > > > >>>>>> For this case, both Serdes are wrappers and user would > > > actually > > > > > only > > > > > > >>>>>> specified wrapped Serdes for the key and value. Can we do > > > > anything > > > > > > >>> about > > > > > > >>>>>> this? Otherwise, there might still be a runtime > > > > > `ClassCastException` > > > > > > >>>>>> that a user cannot easily debug. > > > > > > >>>>>> > > > > > > >>>>>> > > > > > > >>>>>> (2) Nit: The JavaDocs of `Processor#storeSet()` seems to > be > > > > > > incorrect > > > > > > >>>>>> (it says "The names of all connected stores." -- guess > it's > > > c&p > > > > > > >>> error)? > > > > > > >>>>>> > > > > > > >>>>> Yes! Fixed. > > > > > > >>>>> > > > > > > >>>>>> > > > > > > >>>>>> (3) The KIP mentioned to add `Store#changelogTopic()` > > method, > > > > but > > > > > > the > > > > > > >>>>>> output of `TopologyDescription#toString()` does not > contain > > > it. > > > > I > > > > > > >>> think > > > > > > >>>>>> it might be good do add it, too? > > > > > > >>>>>> > > > > > > >>>>> Yes, that's right. I'm going to add to the example as well. > > > > > > >>>>> > > > > > > >>>>>> > > > > > > >>>>>> (4) The KIP also list > > > > > > >>> https://issues.apache.org/jira/browse/KAFKA-9913 > > > > > > >>>>>> but it seems not to address it yet? > > > > > > >>>>>> > > > > > > >>>>> I actually did intent to have it addressed; the proposal > > > > includes: > > > > > > >>>>> > > > > > > >>>>> a. Return the set of source / sink nodes of a sub-topology, > > and > > > > > their > > > > > > >>>>> corresponding source / sink topics could be accessed. > > > > > > >>>>> b. Return the set of stores of a sub-topology, and their > > > > > > corresponding > > > > > > >>>>> changelog topics could be accessed. > > > > > > >>>>> > > > > > > >>>>> The reason I did not choose to just expose the set of all > > > topics > > > > > > >>> directly, > > > > > > >>>>> but indirectly, is stated in the wiki: > > > > > > >>>>> > > > > > > >>>>> "the reason we did not expose APIs for topic names directly > > is > > > > that > > > > > > for > > > > > > >>>>> source nodes, it is possible to have Pattern and for sink > > > nodes, > > > > it > > > > > > is > > > > > > >>>>> possible to have topic-extractors, and hence it's better to > > let > > > > > users > > > > > > >>>>> leveraging on the lower-level APIs to construct the topic > > names > > > > > > >>>>> programmatically themselves." > > > > > > >>>>> > > > > > > >>>>>> > > > > > > >>>>>> (5) As John, I also noticed that `List<String> > > > > > Store#sedersNames()` > > > > > > is > > > > > > >>>>>> not a great API. I am not sure if I understand your reply > > > > thought. > > > > > > >>>>>> AFAIK, there is no exiting API > > > > > > >>>>>> > > > > > > >>>>>>> List<Serde> StoreBuilder#serdes() > > > > > > >>>>>> > > > > > > >>>>>> (cf > > > > > > >>>>>> > > > > > > >>>>> > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/kafka/blob/trunk/streams/src/main/java/org/apache/kafka/streams/state/StoreBuilder.java > > > > > > >>>>> ) > > > > > > >>>>>> > > > > > > >>>>> Ah yes, that's added as part of this KIP. > > > > > > >>>>> > > > > > > >>>>>> > > > > > > >>>>>> (6) Atm, we return `String` type for the Serdes. Do we > think > > > > it's > > > > > > >>>>>> sufficient? Just want to double check. > > > > > > >>>>> > > > > > > >>>>> The reason is that we can only get the serde-name at the > time > > > of > > > > > the > > > > > > >>>>> topology since it may be from config and hence there's no > > serde > > > > > > object > > > > > > >>>>> actually. > > > > > > >>>>> > > > > > > >>>>>> (10) Can we avoid coupling this KIP’s behavior to the > choice > > > of > > > > > > >>> ‘build’ > > > > > > >>>>> method? I.e., can we return the improved description even > > when > > > > > people > > > > > > >>> just > > > > > > >>>>> call ‘build()’? > > > > > > >>>>> > > > > > > >>>>> Yes, as I replied in the above comment to yours, I've > changed > > > my > > > > > mind > > > > > > >>> to > > > > > > >>>>> just return the augmented description no matter of the > > > function; > > > > > and > > > > > > >>> will > > > > > > >>>>> expose toJson() for future compatibilities. I've not yet > > > updated > > > > > the > > > > > > >>> wiki > > > > > > >>>>> yet. > > > > > > >>>>> > > > > > > >>>>>> Clearly, we need a placeholder if no serde is specified. > How > > > > about > > > > > > >>>>> “unknown”, or the name of the config keys, > > > > > > >>>>> “default.key.serde”/“default.value.serde”? > > > > > > >>>>> > > > > > > >>>>> I think if `build(props)` is used, we can use the name of > the > > > > > > >>> configured > > > > > > >>>>> values; otherwise since we do not know the config yet we'd > > have > > > > to > > > > > > use > > > > > > >>>>> "unknown". > > > > > > >>>>> > > > > > > >>>>>> I still have some deep reservation about the > > > ‘build(Parameters)’ > > > > > > >>> method > > > > > > >>>>> itself. I don’t really want to side-track this conversation > > > with > > > > > all > > > > > > my > > > > > > >>>>> concerns if we can avoid it, though. It seems like > > > justification > > > > > > enough > > > > > > >>>>> that introducing dramatically different behavior based in > on > > > > > > seemingly > > > > > > >>>>> minor differences in api calls will be a source of mystery > > and > > > > > > >>> complexity > > > > > > >>>>> for users. > > > > > > >>>>> > > > > > > >>>>>> I.e., I’m characterizing a completely different string > > format > > > as > > > > > > >>>>> “dramatically different”, as opposed to just having a > > > placeholder > > > > > > >>> string. > > > > > > >>>>> > > > > > > >>>>>> (11) Regarding the wrapper serdes, I bet we can capture > and > > > > print > > > > > > the > > > > > > >>>>> inner types as well. > > > > > > >>>>> > > > > > > >>>>> Ack, I can do that. > > > > > > >>>>> > > > > > > >>>>> On Sat, May 2, 2020 at 8:19 AM John Roesler < > > > vvcep...@apache.org > > > > > > > > > > > >>> wrote: > > > > > > >>>>> > > > > > > >>>>>> Hi all, > > > > > > >>>>>> > > > > > > >>>>>> I’ve been sitting on another concern about this proposal. > > > Since > > > > > > >>> Matthias > > > > > > >>>>>> has just submitted a few questions, perhaps I can pile on > > two > > > > more > > > > > > >>> this > > > > > > >>>>>> round. > > > > > > >>>>>> > > > > > > >>>>>> (10) Can we avoid coupling this KIP’s behavior to the > choice > > > of > > > > > > >>> ‘build’ > > > > > > >>>>>> method? I.e., can we return the improved description even > > when > > > > > > people > > > > > > >>> just > > > > > > >>>>>> call ‘build()’? > > > > > > >>>>>> > > > > > > >>>>>> Clearly, we need a placeholder if no serde is specified. > How > > > > about > > > > > > >>>>>> “unknown”, or the name of the config keys, > > > > > > >>>>>> “default.key.serde”/“default.value.serde”? > > > > > > >>>>>> > > > > > > >>>>>> I still have some deep reservation about the > > > ‘build(Parameters)’ > > > > > > >>> method > > > > > > >>>>>> itself. I don’t really want to side-track this > conversation > > > with > > > > > all > > > > > > >>> my > > > > > > >>>>>> concerns if we can avoid it, though. It seems like > > > justification > > > > > > >>> enough > > > > > > >>>>>> that introducing dramatically different behavior based in > on > > > > > > seemingly > > > > > > >>>>>> minor differences in api calls will be a source of mystery > > and > > > > > > >>> complexity > > > > > > >>>>>> for users. > > > > > > >>>>>> > > > > > > >>>>>> I.e., I’m characterizing a completely different string > > format > > > as > > > > > > >>>>>> “dramatically different”, as opposed to just having a > > > > placeholder > > > > > > >>> string. > > > > > > >>>>>> > > > > > > >>>>>> (11) Regarding the wrapper serdes, I bet we can capture > and > > > > print > > > > > > the > > > > > > >>>>>> inner types as well. > > > > > > >>>>>> > > > > > > >>>>>> Thanks again for the KIP! > > > > > > >>>>>> -John > > > > > > >>>>>> > > > > > > >>>>>> > > > > > > >>>>>> > > > > > > >>>>>> On Thu, Apr 30, 2020, at 19:10, Matthias J. Sax wrote: > > > > > > >>>>>>> Guozhang, > > > > > > >>>>>>> > > > > > > >>>>>>> thanks for the KIP! > > > > > > >>>>>>> > > > > > > >>>>>>> Couple of comments/questions. > > > > > > >>>>>>> > > > > > > >>>>>> > > > > > > >>>>>>> > > > > > > >>>>>>> > > > > > > >>>>>>> > > > > > > >>>>>>> -Matthias > > > > > > >>>>>>> > > > > > > >>>>>>> > > > > > > >>>>>>> > > > > > > >>>>>>> > > > > > > >>>>>>> On 4/25/20 1:24 PM, Guozhang Wang wrote: > > > > > > >>>>>>>> Hi John, > > > > > > >>>>>>>> > > > > > > >>>>>>>> Thanks for the review! Replied inline. > > > > > > >>>>>>>> > > > > > > >>>>>>>> On Fri, Apr 24, 2020 at 8:09 PM John Roesler < > > > > > vvcep...@apache.org > > > > > > > > > > > > > >>>>>> wrote: > > > > > > >>>>>>>> > > > > > > >>>>>>>>> Hi Guozhang, > > > > > > >>>>>>>>> > > > > > > >>>>>>>>> Thanks for the KIP! I took a quick look, and I'm really > > > happy > > > > > to > > > > > > >>> see > > > > > > >>>>>> this > > > > > > >>>>>>>>> underway. > > > > > > >>>>>>>>> > > > > > > >>>>>>>>> Some quick questions: > > > > > > >>>>>>>>> > > > > > > >>>>>>>>> 1. Can you elaborate on the reason that stores just > > have a > > > > > list > > > > > > of > > > > > > >>>>>>>>> serdes, whereas > > > > > > >>>>>>>>> other components have an explicit key/value serde? > > > > > > >>>>>>>>> > > > > > > >>>>>>>> > > > > > > >>>>>>>> This is because of the existing API "List<Serde> > > > > > > >>>>>> StoreBuilder#serdes()". > > > > > > >>>>>>>> Although both of its implementations would return two > > serdes > > > > > (one > > > > > > >>> for > > > > > > >>>>>> key > > > > > > >>>>>>>> and one for value), the API is more general to return a > > > list. > > > > > And > > > > > > >>>>>> hence the > > > > > > >>>>>>>> TopologyDescription#Store which gets them directly from > > > > > > >>> StoreBuilder is > > > > > > >>>>>>>> exposing the same API. > > > > > > >>>>>>>> > > > > > > >>>>>>>> 1.5. A side-effect of this seems to be that the > > > > string-formatted > > > > > > >>> serde > > > > > > >>>>>>>>> description is > > > > > > >>>>>>>>> different, depending on whether the serdes are listed > on > > a > > > > > store > > > > > > >>> or a > > > > > > >>>>>>>>> topic. Just an > > > > > > >>>>>>>>> observation. > > > > > > >>>>>>>>> > > > > > > >>>>>>>> > > > > > > >>>>>>>> Yes I agree. I think we can probably change the > > "List<Serde> > > > > > > >>>>>>>> StoreBuilder#serdes()" signature as well (which would > be a > > > > > > breaking > > > > > > >>>>>> change > > > > > > >>>>>>>> though, so we should do that via deprecation), but I'm a > > bit > > > > > > >>> concerned > > > > > > >>>>>>>> since it was designed for future store types which may > not > > > be > > > > of > > > > > > K-V > > > > > > >>>>>> format > > > > > > >>>>>>>> any more. > > > > > > >>>>>>>> > > > > > > >>>>>>>> > > > > > > >>>>>>>>> 2. You mentioned the key compatibility concern in my > > mind. > > > We > > > > > do > > > > > > >>> know > > > > > > >>>>>> that > > > > > > >>>>>>>>> such > > > > > > >>>>>>>>> use cases exist. Namely, our own tests and > > > > > > >>>>>>>>> https://zz85.github.io/kafka-streams-viz/ > > > > > > >>>>>>>>> I'm wondering if we can add a forward-compatible > > > > > machine-readable > > > > > > >>>>>> format > > > > > > >>>>>>>>> to the > > > > > > >>>>>>>>> KIP, so that even though we must break the parsers > right > > > now, > > > > > > maybe > > > > > > >>>>>> we'll > > > > > > >>>>>>>>> never > > > > > > >>>>>>>>> have to break them again. For example, I'm thinking of > a > > > > > "toJson" > > > > > > >>>>>> method > > > > > > >>>>>>>>> on the > > > > > > >>>>>>>>> TopologyDescription that formats the entire topology > > > > > description > > > > > > >>> as a > > > > > > >>>>>> json > > > > > > >>>>>>>>> string. > > > > > > >>>>>>>>> > > > > > > >>>>>>>>> > > > > > > >>>>>>>> Yes, I also have concerns about that (as described in > the > > > > > > >>> compatibility > > > > > > >>>>>>>> section). One proposal I have is that we ONLY augment > the > > > > > toString > > > > > > >>>>>> result > > > > > > >>>>>>>> if the TopologyDescription is from a Topology built from > > > > > > >>>>>>>> `StreamsBuilder#build(Properties)`, which is only > recently > > > > added > > > > > > and > > > > > > >>>>>> hence > > > > > > >>>>>>>> most old usage would not get the benefits of it. But > after > > > > > > thinking > > > > > > >>>>>> about > > > > > > >>>>>>>> this a bit more, I'm now more inclined to just always > > > augment > > > > > the > > > > > > >>>>>> string, > > > > > > >>>>>>>> and also add a `toJson` method in addition to > `toString`. > > > > > > >>>>>>>> > > > > > > >>>>>>>> > > > > > > >>>>>>>>> Thanks again! > > > > > > >>>>>>>>> -John > > > > > > >>>>>>>>> > > > > > > >>>>>>>>> On Fri, Apr 24, 2020, at 00:26, Guozhang Wang wrote: > > > > > > >>>>>>>>>> Hello folks, > > > > > > >>>>>>>>>> > > > > > > >>>>>>>>>> I'd like to start the discussion for KIP-598: > > > > > > >>>>>>>>>> > > > > > > >>>>>>>>>> > > > > > > >>>>>>>>> > > > > > > >>>>>> > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=148648762 > > > > > > >>>>>>>>>> > > > > > > >>>>>>>>>> It proposes to augment the topology description's > > > > sub-classes > > > > > > with > > > > > > >>>>>> store > > > > > > >>>>>>>>>> and source / sink serde information. Let me know what > > you > > > > > think, > > > > > > >>>>>> thanks! > > > > > > >>>>>>>>>> > > > > > > >>>>>>>>>> -- > > > > > > >>>>>>>>>> -- Guozhang > > > > > > >>>>>>>>>> > > > > > > >>>>>>>>> > > > > > > >>>>>>>> > > > > > > >>>>>>>> > > > > > > >>>>>>> > > > > > > >>>>>>> > > > > > > >>>>>>> Attachments: > > > > > > >>>>>>> * signature.asc > > > > > > >>>>>> > > > > > > >>>>> > > > > > > >>>>> > > > > > > >>>>> -- > > > > > > >>>>> -- Guozhang > > > > > > >>> > > > > > > >>> > > > > > > >> > > > > > > >> -- > > > > > > >> -- Guozhang > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > -- Guozhang > > > > > > > > > > > > > > > > > > -- > > > -- Guozhang > > > > > > > > -- > -- Guozhang >