Hi Adrian and Rob, I wish to successfully-close this WGLC but, first, can you confirm that the updates address your concerns?
Thanks, Kent (and Lou) // chair > On Feb 8, 2025, at 2:12 AM, maqiufang (A) > <maqiufang1=40huawei....@dmarc.ietf.org> wrote: > > Hi, Rob, > > Thanks a lot for the review, apologize for the delay. > The authors have submitted -04 > (https://datatracker.ietf.org/doc/html/draft-ietf-netmod-schedule-yang-04, > diff is available at > https://author-tools.ietf.org/iddiff?url2=draft-ietf-netmod-schedule-yang-04) > which incorporates some of your concerns/comments below, please also see my > reply below inline. My co-authors may want to comment more if they want. > > From: Rob Wilton (rwilton) [mailto:rwil...@cisco.com] > Sent: Tuesday, February 4, 2025 6:28 PM > To: James Cumming (Nokia) <james.cumming=40nokia....@dmarc.ietf.org > <mailto:james.cumming=40nokia....@dmarc.ietf.org>>; > draft-ietf-netmod-schedule-y...@ietf.org > <mailto:draft-ietf-netmod-schedule-y...@ietf.org> > Cc: NETMOD WG Chairs <netmod-cha...@ietf.org > <mailto:netmod-cha...@ietf.org>>; netmod@ietf.org <mailto:netmod@ietf.org> > Subject: Re: WG Last Call - A Common YANG Data Model for Scheduling > > Hi authors, WG, > > Sorry for a late WGLC review. I would like to thank the authors working on > this document. Like Adrian, I also think that this document will be useful, > but I do have a few concerns/comments. > > Moderate level comments: > > (1) p 4, sec 3.1. Features > > Refer to Section 3.4 for the use of these features. > > I'm not sure that these YANG features are really useful/helpful. Given this > module only defines groupings, then if modules using these groupings are also > predicated by these features then the same constraint on basic-recurrence or > icalendar-recurrence will affect all modules. > > Will it always be the case that they will want a shared fate, or would it be > better for the modules using these groupings to define their own module > specific feature statements? I.e., this draft could suggest that uses of the > groupings define their own features if needed, or just say nothing at all and > assume that they will do the right thing. > I am unsure about this, currently we have different complex levels of > recurrence representation, features are defined to allow implementations to > declare which one they support (e.g., through YANG-library). You are right > that consumer modules can define their own specific feature statement, but > the initial consideration for defining features here is for better usage > consistency. Does the clarification help? > > > (2) p 7, sec 3.3.1. The "generic-schedule-params" Grouping > > grouping generic-schedule-params: > +-- description? string > > Most of the definitions include a description, but I would have thought that > in many instances the description would likely be superfluous to > requirements. Of course, when the groupings are used the description > statement could be deviated, but I think that it may be better to elide the > description from the groupings and then it could be augmented in during the > uses statements, if needed by the uses of these groupings. > The description parameter could be useful to record any other comments and > details that cannot be available in the structure. The authors had some > internal discussion and we incline to keep this in -04, but would also > welcome any other feedback from the WG. > > (3) p 7, sec 3.3.1. The "generic-schedule-params" Grouping > > +-- time-zone-identifier? sys:timezone-name > +-- validity? yang:date-and-time > +-- max-allowed-start? yang:date-and-time > +-- min-allowed-start? yang:date-and-time > +-- max-allowed-end? yang:date-and-time > > Quite a few of the definitions include both a time-zone-identifier and also > various leaves using the yang:date-and-time. I think that the text > description of these leaves needs to be very explicit regarding what > time-offset should be used, what valid values are, etc. E.g., if I say that > the time-zone is PST but then use data-and-time values at +01:00, then what > does that mean? > > Possibly, you might want to consider introducing a date-and-time-no-zone type > to avoid the ambiguity. > Good point, I agree we fail to make it clear. Some usage requirements of this > parameter have been added in -04 in the section where this parameter is > defined. For example: > “The "time-zone-identifier" parameter, if provided, specifies the time zone > reference [RFC7317] of the local date and time values. This parameter MUST be > specified if any of the date and time values are in the format of local time. > It MUST NOT be applied to date and time values which are specified in the > format of UTC or time zone offset to UTC.” > > Note we did not define a type for date-and-time-no-zone as we still want some > flexibility here to also allow other formats to be used inside this grouping. > (4) p 14, sec 3.3.6. The "recurrence-utc-with-date-times" Grouping > > grouping generic-schedule-params: > ... > grouping period-of-time: > ... > grouping recurrence-basic: > ... > grouping recurrence-utc: > ... > grouping recurrence-with-time-zone: > ... > grouping recurrence-utc-with-date-times: > +-- recurrence-first > | +-- start-time-utc? yang:date-and-time > | +-- duration? uint32 > +-- (recurrence-end)? > | +--:(until) > | | +-- utc-until? yang:date-and-time > | +--:(count) > | +-- count? uint32 > +-- recurrence-description? string > +-- frequency identityref > +-- interval? uint32 > +-- period-timeticks* [period-start] > +-- period-start yang:timeticks > +-- period-end? yang:timeticks > > I found the naming of this group and associated description to be somewhat > confusing. I.e., I can see that this recurrence uses utc and date-times, but > then so does recurrence-utc ... Would it be better if this was called > something like recurrence-utc-with-periods? > > A similar comment applies to recurrence-time-zone-with-date-times? > Sure. Renamed as “recurrence-utc-with-periods” and > “recurrence-time-zone-with-periods” in -04. Related text and examples are > also fixed correspondingly. > > Minor level comments: > > (5) p 7, sec 3.3.1. The "generic-schedule-params" Grouping > > +-- discard-action? identityref > grouping period-of-time: > ... > grouping recurrence-basic: > ... > grouping recurrence-utc: > ... > grouping recurrence-with-time-zone: > ... > grouping recurrence-utc-with-date-times: > ... > grouping recurrence-time-zone-with-date-times: > ... > grouping icalendar-recurrence: > ... > grouping schedule-status: > ... > grouping schedule-status-with-name: > ... > > This module defines a lot of very similar groupings. I am slightly concerned > that this gives too many options and variants for doing it in different ways. > I think that it may be nicer if we offered fewer choices for the recurrence > groups, which would presumably mean that when it is used, it is used in a > more standard way. > > I.e., perhaps recurrence-basic, recurrence-utc, and > recurrence-time-zone-with-date-times would be sufficient, and then the module > using these grouping could deviate remove the parts that they don't need? > I see your concern, currently we have six of recurrence related groupings, > but this is for the consideration of modularity (i.e., a more comprehensive > grouping is the result of using a simple one and then defining more > parameters on top of that) and to accommodate different requirements. > We only have one of them at the very beginning, but then the authors of > tvr-schedule-yang requests a grouping with all parameters using UTC time > format to ensure consistency and machine readability; while authors of > opsawg-scheduling-oam-tests wish to have a time-zone-identifier parameter to > improve readability and maintainability; and also authors of opsawg-ucl-acl > prefer to use icalendar-recurrence to facilitate the enterprise network users > to define the recurrence rule based on week, work day, etc. It’s true that > all these drafts could use the simplest one and then add their own parameters > in their drafts, but these parameters seem generic enough to represent > recurrence rule and should be covered in this draft. > Does this make sense? > (6) p 13, sec 3.3.5. The "recurrence-with-time-zone" Grouping > > grouping generic-schedule-params: > ... > grouping period-of-time: > ... > grouping recurrence-basic: > ... > grouping recurrence-utc: > ... > grouping recurrence-with-time-zone: > +-- recurrence-first > | +-- start-time? yang:date-and-time > | +-- duration? duration > | +-- time-zone-identifier? sys:timezone-name > +-- (recurrence-end)? > | +--:(until) > | | +-- until? yang:date-and-time > | +--:(count) > | +-- count? uint32 > +-- recurrence-description? string > +-- frequency identityref > +-- interval? uint32 > > Does the time-zone-identifier only relate the start-time (because it is under > the recurrence-first container) or also the until time? > Good point. It’s been moved out of the recurrence-first container in -04. > This parameter does not only relate to the start-time, but also until time, > if either of them is declared in the format of local-time. > > Nit level comments: > > (7) p 41, sec 7. Security Considerations > > The "ietf-schedule" module defines a set of types and groupings. > These nodes are intended to be reused by other YANG modules. The > module by itself does not expose any data nodes that are writable, > data nodes that contain read-only state, or RPCs. As such, there are > no additional security issues related to the "ietf- schedule" module > that need to be considered. > > you seem to have an extra space lurking in "ietf- schedule". > Fixed. Thanks a lot! > > Kind regards, > Rob > > > Best Regards, > Qiufang > > From: James Cumming (Nokia) <james.cumming=40nokia....@dmarc.ietf.org > <mailto:james.cumming=40nokia....@dmarc.ietf.org>> > Date: Thursday, 16 January 2025 at 22:39 > To: draft-ietf-netmod-schedule-y...@ietf.org > <mailto:draft-ietf-netmod-schedule-y...@ietf.org> > <draft-ietf-netmod-schedule-y...@ietf.org > <mailto:draft-ietf-netmod-schedule-y...@ietf.org>> > Cc: NETMOD WG Chairs <netmod-cha...@ietf.org > <mailto:netmod-cha...@ietf.org>>, netmod@ietf.org <mailto:netmod@ietf.org> > <netmod@ietf.org <mailto:netmod@ietf.org>> > Subject: [netmod] WG Last Call - A Common YANG Data Model for Scheduling > > All, > > This starts working group last call on the Common YANG Data Model for > Scheduling draft. > > https://datatracker.ietf.org/doc/draft-ietf-netmod-schedule-yang/03/ > > The IPR call prior to WGLC is concluded with no IPR was previously disclosed. > (Thread:https://mailarchive.ietf.org/arch/search/?as=1&email_list=&end_date=&q=subject%3A%28+IPR+call+prior+to+WGLC+-+A+Common+YANG+Data+Model+for+Scheduling%29&qdr=a&start_date=) > > The working group last call ends on January 31st. > > Please send your comments to the working group mailing list. > > Positive comments, e.g., "I've reviewed this document and believe it is ready > for publication", are welcome! > This is useful and important, even from authors. > > Thank you, > James (Document shepherd) > > _______________________________________________ > netmod mailing list -- netmod@ietf.org <mailto:netmod@ietf.org> > To unsubscribe send an email to netmod-le...@ietf.org > <mailto:netmod-le...@ietf.org>
_______________________________________________ netmod mailing list -- netmod@ietf.org To unsubscribe send an email to netmod-le...@ietf.org
