[netmod] AD review of draft-ietf-netmod-schedule-yang-05

[Mahesh Jethanandani <[e-mail]>]

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!

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." 

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.


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.

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 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?

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?

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?

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?

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?

The same comment applies to the grouping 'recurrence-with-time-zone'.

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.

"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.

-------------------------------------------------------------------------------
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.

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
[e-mail]