Hi,
Let me start by thanking the authors because I believe this YANG model
will be a useful building block for a number of operational features
that rely on the ability to schedule functions within the network.
I'd like to see this document progress through WGLC, but I did find a
lot of (relatively small) comments during my review. I'm sorry, but I
did not check my comments for duplication of Reshad's review.
I did not review the examples, but congratulations on not having any
wrapping lines!
Cheers,
Adrian
= Comments =
I note some tension between "scheduling" which is a planning process,
and "reporting" which makes a statement about what has happened in the
past.
The document sems to mainly live up to its title (i.e., scheduling), but
I find text such as (in A.9):
At the time of 12:15 PM, 2025-12-01 (UTC), the recurring event
occurred at (note that occurrence at 10:20 AM is excluded): 9:00,
9:20, 9:40, 10:00, 10:40, 11:00, 11:20, 11:40, 12:00. The last
occurrence was at 12:00, the upcoming one is at 12:20.
It seems a very small step from reporting what scheduled things happened
to reporting what unscheduled things happened. I'm not sure that there
is anything to be done here, but it is possible that the Abstract and
Introduction need to call out more specifically the fact that the module
can also report what has happened as well as what is to happen.
---
The Abstract says that the granularity levels vary from basic to
advanced. Is that true? Or is it, as stated in 3.1, the representation
of the recurrence rule that varies from basic to advanced?
---
Introduction has:
Section 5 discusses relationship with the managed objects defined in
[RFC3231].
Very good, but a little hint would be helpful. Such as:
Section 5 discusses the relationship with the Management Information
Base (MIB) managed objects for scheduling management operations
defined in [RFC3231].
---
Instead of Section 3 being called "Groupings" it might be more helpfully
named "Scheduling Groupings".
Similarly, Appendix A might be better as "Examples of Scheduling Format
Representation"
---
I hate that 5545 used "SECONDLY" and "MINUTELY". It makes me twitch
every time I read them. But you are right to use the terms defined
there. I do wonder whether using upper case, as in 5545, might make the
normal reader less ill.
Note that in 3.2 you use "per second, per minute, etc.". Perhaps this
should be "secondly, minutely, etc.".
---
3.1 says:
Refer to Section 3.4 for the use of these features.
But I think these features are discussed in subsections of 3.3. It's
true that 3.4 is on "Feature Use" but it only contains 8 lines of text.
---
I think maybe re-order "frequency-type" and "schedule-type" because
frequency is only relevant for recurring schedules.
This could also apply in the module, itself.
---
As I understand "schedule-state" it is used to read the current state
of a schedule, but also to control it. I really think you should not
overload and confuse this. Just as we would have done in SMI, you should
have two objects: intended-schedule-state and current-schedule-state.
---
The description of "discard-action" is mangled in the document,
I find:
3.2
* "discard-action": Specifies the action to perform when a schedule
is discarded (e.g., generate a warning or an error message).
3.3.1
The "discard-action" parameter specifies the action if a requested
schedule is considered inactive or out-of-date.
6
identity discard-action {
description
"Indicates that a schedule will be discarded.";
}
6
leaf discard-action {
type identityref {
base discard-action;
}
description
"Specifies the behavior when a schedule is discarded when
enforcing the guards in this grouping or it is received
out-of-date.";
}
Now, I'll grant that these are *similar* but they don't feel to be the
same.
I'd suggest:
- In 3.2, spend a little time explaining what "discarded" means.
Perhaps the text from 3.3.1 would do.
- Fix the description of the discard-action identity, as this is
definitely wrong.
- Consider whether the description of the discard-action leaf appears
to limit the cases of discard leaving other types of discard
unaddressed.
- Worry about having a leaf with the same name as the identity.
---
3.3.1 has:
The "time-zone-identifier" parameter, if provided, specifies the time
zone reference of the date and time values with local time format.
The use of "with local time format" made me think that the format of the
time-zone-identifier was a local matter. But this is not, I think, what
you mean. Perhaps:
The "time-zone-identifier" parameter, if provided, specifies the time
zone reference [RFC7317] of the local date and time values.
---
3.3.1 has:
The "validity" parameter specifies the date and time after which a
schedule will be considered as invalid. It determines the latest
time that a schedule can be executed by a system and takes precedence
over similar attributes that are provided at the schedule instance
itself.
Now, consider a schedule event that takes a finite amount of time to be
executed. Do you mean that execution must complete before or at the
validity time or that execution must start before or at the validity
time?
---
3.3.2
The "period-of-time" grouping (Figure 3) represents a time period
using either a start ("period-start") and end date and time ("period-
end"), or a start ("period-start") and a positive time duration
("duration").
Why is period-end described as "end date and time" but period-start is
just described as "start"? Surely it should be "start date and time".
---
Duration
I came to this when reading 3.3.2
The "period-of-time" grouping (Figure 3) represents a time period
using either a start ("period-start") and end date and time ("period-
end"), or a start ("period-start") and a positive time duration
("duration"). For the first format, the start of the period MUST be
before the end of the period.
There is an obvious imbalance that made me think "can duration be zero?
and if so, why must the start be before the end (and not before or equal
to)?
So I searched "duration" in the document and I found a real mixture. It
may be useable, but I wonder why it is necessary to have all the
following ways of expressing duration.
3.3.4 and 3.3.6
| +-- duration? uint32
Which allows zero.
6
typedef duration {
type string {
pattern '((\+)?|\-)P((([0-9]+)D)?(T(0[0-9]|1[0-9]|2[0-3])'
+ ':[0-5][0-9]:[0-5][0-9]))|P([0-9]+)W';
}
Which allows +/-/0
6
case duration {
description
"A period of time is defined by a start and a positive
duration of time.";
leaf duration {
type duration {
pattern 'P((([0-9]+)D)?(T(0[0-9]|1[0-9]|2[0-3])'
+ ':[0-5][0-9]:[0-5][0-9]))|P([0-9]+)W';
}
Which is +/0 although the text says it is a positive duration.
---
3.3
Note that per Section 4.13 of
[I-D.ietf-netmod-rfc8407bis], no "default" substatement is used here
because there are cases (e.g., profiling) where the use of the
default is problematic.
This feels like [I-D.ietf-netmod-rfc8407bis] is being used as a
normative reference. Shouldn't be an issue as that draft is a little
ahead in the processing queue.
---
3.3.4
The "duration" parameter specifies, in units of seconds, the time
period of the first occurrence. Unless specified otherwise, the
"duration" also applies to subsequent recurrence instances.
Unless specified otherwise how?
---
3.3.4
Note that the "interval" and "duration" cover two distinct properties
of a schedule event. The interval specifies when a schedule will
occur, combined with the frequency parameter; while the duration
indicates how long an occurrence will last.
Clear.
But what happens if the interval is shorter than the duration? Is that
allowed?
---
Count
The various textual descriptions and tree diagrams made me see that 0
was allowed (unit32) but that there was an assumption that the first
occurrence would always occur. Only when I got the module itself did I
find
leaf count {
type uint32;
description
"The positive number of occurrences at which to
terminate the recurrence.";
}
Perhaps the text (in all the cases) should include "MUST be GTE 1"
---
Direction
Not totally happy that the scoping -53..-1|1..53 can create some
confusion of the 53rd Monday in the month, etc.
---
failure-counter
Do we need to know when this counter was last reset?
---
4.
* Schedules received with a starting time in the past with respect
to current time SHOULD be ignored.
Please give the alternative "MAY" behaviour.
---
7. has:
This section uses the template described in Section 3.7 of
[I-D.ietf-netmod-rfc8407bis].
This is slightly risky with the draft as an Informational reference
because the template might change under your feet and this statement
would cease to be true.
I think that either you remove this statement (and, indeed, why is it
there except because you are tired of justifying the content and
format of Security Considerations sections of YANG documents that you
write!) or make the reference normative and risk a delay while that
draft completes the process as well as needing to come back later to
check that you are still using the correct template.
---
I believe that section 7 is missing clear instructions to future module
that make use of this module.
= Nits =
Section 2
s/entity that host/entity that hosts/
---
I found 3.3 a bit over the top.
Is Figure 1 in any way helpful?
Having given the cross-references to the section that describes each
grouping, do you need to say "Each of these groupings is presented in
the following subsections."
---
3.3.1
s/handles the schedule requests/handles schedule requests/
---
Throughout, please consider s/invalid/not valid/
---
2.2
Interval: Refers to an integer that specifies at which intervals a
recurrence rule repeats. Values are taken from "INTERVAL" rule in
Section 3.3.10 of [RFC5545].
3.3.3
Consistent with Section 3.3.10 of [RFC5545], the interval
("interval") represents at which intervals the recurrence rule
repeats.
Is that really "intervals" or should it be "interval"?
---
3.3.6
s/must not/MUST NOT/
s/e.g./i.e./
---
4.
OLD
* The second MUST have the value "60" at the end of months in which
a leap second occurs for date and time values.
NEW
* The second for date and time values MUST have the value "60" at
the end of months in which a leap second occurs.
END
---
5.
OLD
Despite no data nodes are defined in this document, Table 1 lists how
main objects in the DISMAN-SCHEDULE-MIB can be mapped to YANG
parameters.
NEW
Although no data nodes are defined in this document, Table 1 lists
how the main objects in the DISMAN-SCHEDULE-MIB can be mapped to YANG
parameters.
END
---
From: James Cumming (Nokia) <james.cumming=40nokia....@dmarc.ietf.org>
Sent: 16 January 2025 22:38
To: draft-ietf-netmod-schedule-y...@ietf.org
Cc: NETMOD WG Chairs <netmod-cha...@ietf.org>; 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
<https://mailarchive.ietf.org/arch/search/?as=1&email_list=&end_date=&q=subj
ect%3A%28+IPR+call+prior+to+WGLC+-+A+Common+YANG+Data+Model+for+Scheduling%2
9&qdr=a&start_date=>
&email_list=&end_date=&q=subject%3A%28+IPR+call+prior+to+WGLC+-+A+Common+YAN
G+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
To unsubscribe send an email to netmod-le...@ietf.org
