Hi, Mahesh, Thanks a lot for your review. The authors have made some changes based on your comments which can be found at: https://netmod-wg.github.io/schedule-yang/draft-ietf-netmod-schedule-yang.html, and the diff is available here: https://author-tools.ietf.org/api/iddiff?doc_1=draft-ietf-netmod-schedule-yang&url_2=https://netmod-wg.github.io/schedule-yang/draft-ietf-netmod-schedule-yang.txt.
Please also see below for responses to your comments. From: Mahesh Jethanandani [mailto:[email protected]] Sent: Thursday, May 15, 2025 8:13 AM To: [email protected] Cc: NETMOD Group <[email protected]> Subject: AD review of draft-ietf-netmod-schedule-yang-05 I would like to thank the authors for working on this document. I think it is a much-needed piece of work. I will also note that the several set of examples the authors have provided as part of the document have been truly helpful. Thank you! Thanks;-) I want to call out the review done by Reshad from a YANG doctor's perspective. Thanks for combing through the details. Here are some of my comments on the document. Hope they go towards improving the document further. "Abstract", paragraph 0 > This document defines a common schedule YANG module which is designed > to be applicable for scheduling purposes such as event, policy, > services, or resources based on date and time. For the sake of > better modularity, the module includes a set of recurrence related > groupings with varying levels of representation (i.e., from basic to > advanced) to accommodate a variety of requirements. It also defines > groupings for validating requested schedules and reporting scheduling > status. The document defines common types and groupings for scheduling purposes. It is defining these groupings to be used in some other context, such as events, policy, services, etc. In other words, it is not defining a "scheduling" module per se, as the abstract asserts. Can that be made explicit by saying something like? "This document defines common types and groupings that are meant to be used for scheduling purposes by events, policy, or resources based on date and time." Fixed as suggested. Section 1, paragraph 0 > This document defines a common schedule YANG module ("ietf-schedule") > that can be used in several scheduling contexts, e.g., (but not > limited to) [I-D.ietf-opsawg-ucl-acl], > [I-D.ietf-opsawg-scheduling-oam-tests], and > [I-D.ietf-tvr-schedule-yang]. The module includes a set of reusable > groupings which are designed to be applicable for scheduling purposes > such as event, policy, services or resources based on date and time. > It also defines groupings for validating requested schedules and > reporting scheduling status. I am not an expert on the use of "date and time" in implementations and how operators use them, but this model does raise the question of whether the document should use one definition of 'date-and-time' along with offsets rather than allow the definition of local time and timezone. Having support for the latter means any use of 'date-and-time' in the model has to check whether a timezone has been specified or not, except when it is within "recurrence-utc". It would also greatly simplify the number of groupings by getting rid of *-with-time-zone groupings. Finally, RFC 9557 says this for Local Time. Because the daylight saving rules for local time zones are so convoluted and can change based on local law at unpredictable times, true interoperability is best achieved by using Coordinated Universal Time (UTC). This specification does not cater to local time zone rules. When the authors first design the schedule yang model, we think it would be good to align with the IETF's existing definitions of period and recurrence, namely RFC5545 which defines the Calendar data format and also supports both UTC and local time with time zone format. And also some follow-up discussion with different authors of consumer drafts (e.g., tvr-schedule-yang, opsawg-scheduling-oam-tests) of schedule yang definition shows that there are different requirements for schedule format representation in different cases, and they wish to make a distinction between human readability vs. machine readability. Though utc format is favorable for interoperability, I think a local-time + time zone reference still has wide use in a lot of cases, e.g., timestamps in a lot of audits and logs are usually based on the local time of the device, and this document is intended to be common to accommodate different requirements. I think the quoted text is from RFC 3339, not RFC 9557 which extends the format with additional information including a time zone and fixes the issue of daylight saving time transitions mentioned above. Hopefully this clarifies the rationale behind the authors’ preference to keep the current definition as it is. Section 3.3.1, paragraph 3 > grouping period-of-time: > ... > grouping recurrence-basic: > ... > grouping recurrence-utc: > ... > grouping recurrence-with-time-zone: > ... > grouping recurrence-utc-with-periods: > ... > grouping recurrence-time-zone-with-periods: > ... > grouping icalendar-recurrence: > ... > grouping schedule-status: > ... > grouping schedule-status-with-name: > ... Is there a way to not show the groupings that are not being discussed in this section, or are discussed elsewhere? If other groupings need to be referenced, a reference to the section would probably be better. Fixed. Section 3.3.1, paragraph 5 > 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. If this is a MUST, why is the parameter not marked mandatory? And because of that, it shows up as optional in the tree diagram. I guess it should not be mandatory as it is uncertain whether the date-and-time formats specified in the grouping is in the format of UTC or local time. I also note that I read the thread on GitHub about providing a default and mandatory statement. I wish the discussion had been had on the mailing list and not in GitHub, lest somebody else had comments. I do agree with Qiufang that the default statement and mandatory statement should be provided where applicable. I also note that Reshad brought up a similar point in his YANG doctors' review. If the grouping is reused in multiple places, those definitions for default or mandatory can be refined for the local purpose. Putting them in a description statement does not make it normative. Further, is it not that configured values override default values, and not the other way around? I think there are two questions that were hit. The first is whether the grouping should define default/mandatory statement. I still think that there are some parameters that must have a default value or be defined as mandatory, otherwise the behavior will be unspecified. But I think it would be better for the use-statement to refine with a default/mandatory statement, as it is unsure the default value will be applicable to all cases, and it would be problematic in some cases where a default value is not needed (there is no way to remove a default statement from a grouping). FWIW, section 4.13 of RFC 8407bis also states that * Do not include a "default" substatement on a leaf or choice unless the value applies on all possible contexts. While the discussion with YangDoctor is that, Reshad understands it might be problematic to have default statement in generic groupings, but he is unsure whether a specific parameter should specify the default value in the description statement without a default statement. This is the one I think it would be good to have more discussion and feedback from the WG. To me, this feels to provide a safeguard to understand the default behavior if neither a default or mandatory statement is refined for a use-statement, and for those that really dislike this default value (they want a new value or don’t need the default value at all), it is always possible for the use-statement to refine the description. That is why I felt the current handling has some merit, but also happy to hear from others. Section 3.3.1, paragraph 5 > The "validity" parameter specifies the date and time after which a > schedule will not be considered as valid. It determines the latest > time that a schedule can be started to execute by a system and takes > precedence over similar attributes that are provided at the schedule > instance itself. Looking at this definition and the example in A.1, it is not entirely clear what the difference between 'validity' and 'max-allowed-end' is, other than the fact that one is a 'discard-action' and the other is a 'warning'. Is it that one is allowed to be scheduled but ignored when the validity is not true, while in the other case it is not even scheduled? Sorry for being unclear. Yes, this is one of the difference, and the other difference is that the “validity” determines the max allowed start time to execute a schedule independent of when it ends, while “max-allowed-end” is about the max allowed end time of the last occurrence. We hope these different safeguards give sufficient flexibility for future use. There are some slight rewording of these two parameter definition, please have a review of it and let us if it is clearer. Section 3.3.2, paragraph 1 > The "period-of-time" grouping (Figure 2) represents a time period > using either a start date and time ("period-start") and end date and > time ("period-end"), or a start date and time ("period-start") and a > non-negative time duration ("duration"). For the first format, the > start of the period MUST be no later the end of the period. If > neither an end date and time ("period-end") nor a duration > ("duration") is indicated, the period is considered to last forever > or as a one-shot schedule. If the duration ("duration") value is 0 > or the end time ("period-end") is the same as the start time > ("period-start"), the period is also considered as a one-shot > schedule. If no end date and time or a duration is specified, how does one distinguish between it being a last forever schedule or a one-shot schedule? This is a good point. The update have made a distinction between one-shot and a period lasting forever. one-shot: The schedule will trigger an action that has either the duration specified as 0 or the end time specified the same as start time, and then the schedule will disable itself period: …If neither an end nor a duration is indicated, the period is considered to last forever. Better? Section 3.3.4, paragraph 5 > The "duration" parameter specifies, in units of seconds, the time > period of the first occurrence. Unless specified otherwise (e.g., > described in the "description" statement), the "duration" also > applies to subsequent recurrence instances. When unspecified, each > occurrence is considered as immediate completion or hard to compute > an exact duration. Two comments. A description statement is hardly a place to specify how the parameter should be treated. Can we make it normative that duration applies to subsequent recurrence instances? We would like this to be the default behavior, and future work may define parameters to make other cases normative, so it has been fixed as: Unless specified otherwise (e.g., through additional augmented parameters), the "duration" also applies to subsequent recurrence instances. Also, the last statement is not clear to me. Did you mean to say that if duration is unspecified, it is hard to compute the exact duration and therefore is treated as immediate completion? Not really, these are two different cases that make it possible for the duration to be unspecified. Some examples are given to make it easier to understand: When unspecified, each occurrence is considered as immediate completion (e.g., execute an immediate command that is considered to complete quickly) or hard to compute an exact duration (e.g., run a data analysis script whose execution time may depend on the data volume and computation resource availability). The same comment applies to the grouping 'recurrence-with-time-zone'. Fixed as well. Section 3.3.4, paragraph 5 > The repetition can be scoped by a specified end time or by a count of > occurrences, indicated by the "recurrence-end" choice. The value of > the "count" node MUST be greater than 1, the "start-time-utc" value > always counts as the first occurrence. Can the model carry a must statement to enforce the count value greater than 1? The same comment applies to other groupings where 'count' parameter is used. Yes. Good comment. but I think a range "1..max" statement is simper to restrict the use of positive integers. Agreed? "Appendix A.", paragraph 0 > This section provides some examples to illustrate the use of the > period and recurrence formats defined in Section 6. The following > modules are used for illustration purposes: It would help to explain what each example is doing in this section. The YANG module in Appendix A doesn’t have a lot of meaning, it just provide a mean for the JSON representation to be able to be validated by tools like yanglint based on some previous comments from reviewers. We believe other examples have already been illustrated with its use purposes. ------------------------------------------------------------------------------- NIT ------------------------------------------------------------------------------- All comments below are about very minor potential issues that you may choose to address in some way - or ignore - as you see fit. Some were flagged by automated tools (via https://github.com/larseggert/ietf-reviewtool), so there will likely be some false positives. There is no need to let me know what you did with these suggestions. Thanks a lot, some nits below are fixed, while others are considered as false positives by authors. Best Regards, Qiufang Section 3.3.9, paragraph 0 > The "schedule-status" and "schedule-status-with-name" groupings > (Figure 9) define common parameters for scheduling management/status > exposure. The "schedule-status-with-name" grouping has the same > structure as "schedule-status" but with an additional parameter to > identify a schedule "schedule-id". Both structures are defined in > the module to allow for better modularity and flexibility. Would it better to call the grouping "schedule-status-with-id"? "A.1.", paragraph 0 > Figure 10 illustrates the example of a requested schedule that needs > to start no earlier than 08:00 AM, January 1, 2025 and end no later > than 8:00 PM, January 31, 2025 (Beijing time). Schedule requests > that fail to meet the requirements are ignored by the system as > indicates by "discard-action". s/indicates/indicated/ Section 1.1, paragraph 5 > on. Frequency: Characterizes the type of a recurrence rule. Values are taken > ^^^^^^^^^ If "type" is a classification term, "a" is not necessary. Use "type of". (The phrases "kind of" and "sort of" are informal if they mean "to some extent".). Section 2, paragraph 9 > * "schedule-type": Indicates the type of a schedule. The following types are > ^^^^^^^^^ If "type" is a classification term, "a" is not necessary. Use "type of". (The phrases "kind of" and "sort of" are informal if they mean "to some extent".). Section 3.3.1, paragraph 7 > ng'. Is it that one is allowed to scheduled but ignored when the validity is > ^^^^^^^^^ The verb after "to" should be in the base form as part of the to-infinitive. A verb can take many forms, but the base form is always used in the to-infinitive. "I", paragraph 2 > rmat, the start of the period MUST be no later the end of the period. If neit > ^^ Did you mean "not"? Section 9, paragraph 15 > ription "Example of a module defining an scheduled based backup operation."; > ^^ Use "a" instead of "an" if the following word doesn't start with a vowel sound, e.g. "a sentence", "a university". Section 9, paragraph 18 > specification, only applies when schedule:basic-recurrence feaure is suppor > ^^^^^^^^ The conjunction "when" requires the past participle "scheduled". Or did you mean "you schedule"? Section 9, paragraph 20 > specification, only applies when schedule:icalendar-recurrence feaure is su > ^^^^^^^^ The conjunction "when" requires the past participle "scheduled". Or did you mean "you schedule”? Thanks Mahesh Jethanandani [email protected]<mailto:[email protected]>
_______________________________________________ netmod mailing list -- [email protected] To unsubscribe send an email to [email protected]
