Hi Qiufang, Many thanks for your deep review. See my feedback inline.
On Tue, Jan 17, 2023 at 11:23 AM maqiufang (A) <maqiufa...@huawei.com> wrote: > Hi, all > > > > I had a chance to take a deeper look at this -02 version — not the other > parts, but only the YANG model related to the data-source configuration, > which I believe is the section 5.3.1 and Appendix A.1 in the current draft. > Following are some contributor thoughts: > > · I like the idea that we only define a very general list now and > leave the parameters specific to each data source to be extended in future, > while I am not convinced that the update-policy choice defined here is > proper. My intuition is that for reactive mode, the alto server waits data > feed without needing to poll. It could receive data as soon as it changes, > but periodic collection could also be another mode. > I don't fully understand the feed-interval property in reactive mode. The data-source list actually configures a list of data source listeners in the ALTO server. Should this property be configured by the data source side, instead of the ALTO server side? Do you have any example of how the feed-interval is used by a concrete data source listener? > I question whether a Boolean type of reactive leaf is sufficient, and what > if a false value is configured here? I would rather use an empty type. > Make sense. I like this change. > · I would like the update-policy choice to be defined outside > data-source list (but not very strong feeling), it is not the property of > data-source itself, but how the alto server get data from data-source. > > If we move update-policy outside, could someone configure a data source without configuring its update-policy? If so, how does the ALTO server decide how to get data updates? > That said, my suggested data-source tree diagram is following: > > … > > +--rw data-source* [source-id] > > | +--rw source-id string > > | +--rw source-type identityref > > | +--rw (source-params)? > > +--rw data-update-policy* [source-id] > > | +--rw source-id -> ../../data-source/source-id > > | +--rw (update-policy)? > > | +--:(reactive) > > | | +--rw (reactive-mode)? > > | | +--:(on-change) > > | | | +--rw on-change empty > > | | +--:(period) > > | | +--rw feed-interval uint32 > > | +--:(proactive) > > | +--rw poll-interval uint32 > > … > > · Regarding the Appendix part, it seems to me that some > parameters are still missing. The following tree diagram for a > yang-datastore source example could be for the authors’ reference: > > module: example-ietf-alto-data-source > > augment /alto:alto-server/alto:data-source/alto:source-params: > > +--:(yang-datastore) > > +--rw source-server > > +--rw name? inet:domain-name > > +--rw datastore identityref > > +--rw target-paths* [name] > > | +--rw name string > > | +---u yp:selection-filter-types > > +--rw protocol* identityref > > +--rw restconf > > | +---u rcc:restconf-client-app-grouping > > +--rw netconf > > +---u ncc:netconf-client-app-grouping > > > Thanks for the improvement. Just a small question. What is the usage of the name property of the source-server? Who will reference this name? Is this the duplicate of the source-id? > The difference between *conf-client-app-grouping and > *conf-client-listen-stack-grouping is that the former also supports > initiating connections to servers proactively, which I believe is the more > common case. > Good suggestion. I like this change. > > > All related YANG modules are attached. > Many thanks. I will consider your proposed changes and merge them into the new revision. btw. We have received a lot of very good concrete editing comments and suggestions. We really appreciate your contributions. Would you like to become a coauthor of this document? > > > Best Regards, > > Qiufang > Thanks, Jensen
_______________________________________________ alto mailing list alto@ietf.org https://www.ietf.org/mailman/listinfo/alto
