Subject: [yang-doctors] Yangdoctors early review of draft-ietf-netmod-schedule-yang-02
From: Reshad Rahman via Datatracker <[email-masked]>
To: <[email-masked]>
Cc: [email-masked], [email-masked]
Date: Thu, 03 Oct 2024 12:31:49 -0700
List-Id: Email list of the yang-doctors directorate <yang-doctors.ietf.org>

Reviewer: Reshad Rahman
Review result: On the Right Track

Reviewer: Reshad Rahman
Review result: Almost Ready

Hi all,

This is an early YD review of -02.

- My first impression of the document is that it seems unnecessarily big, why
all these groupings for something as simple as a schedule :-) On further
reading, I do now understand the reason, all the knobs and
belle-and-whistles... The abstract and section 3.1 do mention "basic,
intermediate and advanced versions of recurrence related groupings". But there
is no further mention of which ones are basic/intermediate/advanced. There is a
basic-recurrence feature defined but it is not clear whether that is meant for
only the basic groupings or ... Please consider in section 3.3 whether each
grouping should be tagged as basic/intermediate/advanced and whether the
features should be defined accordingly.

- 3.1 mentions 2 features basic-recurrence and icalendar-recurrence. Is it
possible that one or the other recurrence feature may be supported for some
scheduled items but not for all. e.g. both supported for disk backups but only
basic-recurrence supported for pings to a central controller. When implementing
a standard (e.g. IETF) YANG, a vendor can use deviations to work around that.
Worth adding some text on this? I am also not sure whether it makes sense to
have those features.

- Section 3.1: the feature names have the -supported suffix. This is a personal
preference, but I think the "supported" part is implied for a feature and not
needed in the feature names.

- Section 3.2: one-shot is clear but the difference between period and
recurrence is not.

- Sections 3.3.X, I am not sure why all the other groupings are listed. e.g.
3.3.1 is about "generic-schedule-params", why list all the other groupings in
Figure 2?

- Section 3.3.1, what is the difference between validity and max-allowed-end,
not clear to me.

- Section 3.3.3, should frequency be frequency-unit? Strictly speaking, that's
an interval-unit and not a frequency-unit? It does seem odd to me to have
frequency and interval in the same grouping... And not a fan of identities such
as "daily", "minutely", "secondly": although those are English words I don't
think they mean what you're trying to convey here. But if you rename frequency
to interval-unit, you can use "day", "hour", "minute", "second" etc for
interval-type (renamed from frequency-type).

- Section 3.3.3 v/s 3.3.4, intuitively from "recurrence" to "recurrence-utc" I
expected the change to be just wrt use of UTC. Consider renaming "recurrence"
to "recurrence-basic" since it is basic with just a description and an
interval/frequency.

- Wrt UTC, some nodes have "utc" as prefix and others as suffix. Be consistent
and my preference is for suffix e.g. start-time-utc (instead of utc-start-time).

- Section 3.3.X, be consistent in the names. e.g if UTC uses start-time-utc,
then 3.3.5 should use start-time (not date-time-start).

- Section 3.3.X, many names have recurrence- as prefix e.g. recurrence-first,
recurrence-bound, recurrence-description. Best practice is to remove the
recurrence- prefix and put all these nodes in a recurrence container. You might
to rework the groupings a bit but it should be straightforward.

- Sections 3.3.4 and 3.3.5, not clear to me why UTC is deemed to be for machine
readability and with-time-zone for human readability.

- Section 3.3.4: what happens if duration > interval? I thought I saw some text
on this but can't find it.

- recurrence-bound, I don't understand the use of the word "bound" here, is it
as in "boundary"? Maybe call it limit?

- discard-action does not mention how the warning/error message is generated,
is it a syslog? How about using an alarm (RFC8632) as another option?

- I don't see must-statement that period-end > period-start in the YANG,
although it is mentioned in the text.

- The examples in appendix A are all based on the groupings. But since the
groupings will not be used in a stand-alone way, I think the examples should
illustrate a usage of the groupings. For example, the examples could be based
on the example YANG modules in Appendix B.