Hi Med,
I looked at PRs #49 and #52, the changes look good to me.
Regards,Reshad.
    On Wednesday, October 9, 2024 at 03:13:04 AM EDT, 
[email protected] <[email protected]> wrote:  
 
  
Hi Reshad,
 
  
 
Thanks for the follow-up. We will release a new version with the changes SOON.
 
  
 
Please see inline for more context.
 
  
 
Cheers,
 
Med
 
  
 
Ps: removed ACKed items.
 
  
 
De : Reshad Rahman <[email protected]> 
Envoyé : mardi 8 octobre 2024 23:15
À : [email protected]; BOUCADAIR Mohamed INNOV/NET 
<[email protected]>
Cc : [email protected]; [email protected]
Objet : Re: Yangdoctors early review of draft-ietf-netmod-schedule-yang-02
 
  
 
  
 
Hi Med,
 
  
 
Thanks for the prompt response. Please see inline <RR> (where no explicit 
response, default is ack).
 
  
 
On Friday, October 4, 2024 at 04:33:51 AM EDT, <[email protected]> 
wrote:
 
  
 
  
 
Hi Reshad,

Thank you for the review. 

The diff to track the changes made so far can be found here: 
https://author-tools.ietf.org/api/iddiff?url_1=https://netmod-wg.github.io/schedule-yang/draft-ietf-netmod-schedule-yang.txt&url_2=https://netmod-wg.github.io/schedule-yang/reshad-review/draft-ietf-netmod-schedule-yang.txt

Please see inline for more context. 

I let my co-authors further comment as appropriate.

Cheers,
Med

> -----Message d'origine-----
> De : Reshad Rahman via Datatracker <[email protected]>
> Envoyé : jeudi 3 octobre 2024 21:32
> À : [email protected]
> Cc : [email protected];[email protected]
> Objet : Yangdoctors early review of draft-ietf-netmod-schedule-
> yang-02
> 
> 
> Reviewer: Reshad Rahman
> Review result: On the Right Track
> 
> Hi all,
> 

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

[Med] The use of one or both in the same module is specific to the context 
where the groupings are used. This is why we do say the following:

  Implementations may support a basic
  recurrence rule or an advanced one as needed, by declaring different
  features.  Whether only one or both features are supported is
  implementation specific and depend on specific scheduling context.

Please note that we provided an example where both are used.
 

<RR> I did see the example and that is actually what triggered the question.
 
  
 
The example for scheduled backups has this:
          container basic-recurrence-schedules {            if-feature 
schedule:basic-recurrence-supported;            description              "Basic 
recurrence schedule specification, only applies when               
schedule:basic-recurrence-supported feaure is supported.";            leaf 
schedule-id {              type string;              description                
"The schedule identifier for this recurrence rule.";            }            
uses schedule:recurrence;           }             container 
icalendar-recurrence-schedules {            if-feature 
schedule:icalendar-recurrence-supported;            description              
"Basic recurrence schedule specification, only applies when               
schedule:icalendar-recurrence-supported feaure is               supported.";    
        leaf schedule-id {              type string;              description   
             "The schedule identifier for this recurrence rule.";            }  
             uses schedule:icalendar-recurrence;          } 
  
 
Let's say the device has another module for scheduled pings (based on example 
above):
          container basic-recurrence-ping-schedules {            if-feature 
schedule:basic-recurrence-supported;            description              "Basic 
recurrence schedule specification, only applies when               
schedule:basic-recurrence-supported feaure is supported.";            leaf 
schedule-id {              type string;              description                
"The schedule identifier for this recurrence rule.";            }            
uses schedule:recurrence;           }             container 
icalendar-recurrence-ping-schedules {            if-feature 
schedule:icalendar-recurrence-supported;            description              
"Basic recurrence schedule specification, only applies when               
schedule:icalendar-recurrence-supported feaure is               supported.";    
        leaf schedule-id {              type string;              description   
             "The schedule identifier for this recurrence rule.";            }  
             uses schedule:icalendar-recurrence;          }    <RR> How would 
the device indicate e.g that it supports icalendar-recurrence-schedules but not 
icalendar-recurrence-ping-schedules? [Med] If the base schedule features are 
not sufficient, and such control is needed for a specific context, the device 
module can define dedicated features for that.      Not via the feature since 
both use the same feature in the if-feature statement. And the feature support 
doesn't depend on the context afaik, it is either supported or not supported. 
So I think we'd need to define features for where the groupings are used and 
these features would depend on the features defined in this document?   

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

[Med] The period is similar to one-shot with the exception that it does not 
disable itself once the scheduled action is terminated. Recurrence is more a 
schedule that occurs many times (e.g., periodic).
 
  
 <RR> This subtlety, i.e. period v/s recurrence, still escapes me. If 
recurrence is periodic, then it sounds a lot like "period" :-) If it's clear 
for everyone, maybe I need to look at the document again... But some text in 
3.2 may help. 
  
 
[Med] recurrence is more generic than “periodic”. Added some text to clarify 
this:https://github.com/netmod-wg/schedule-yang/pull/49/files
 

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

[Med] These cover two distinct aspects of activating a schedule (start vs. 
end). Can you please let me know what is not clear in the following text:

  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.

And

  The "max-allowed-end" parameter specifies the maximum allowed end
  time of the last occurrence.  A requested schedule will be rejected
  if the end time of last occurrence is later than the configured "max-
  allowed-end" value.

Thanks.


 <RR> What would help confirm my understanding, or not, is an example with both 
in the appendix. Thanks. [Med] Noted: 
https://github.com/netmod-wg/schedule-yang/issues/50  
  
 
>
> - 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).
> 

[Med] We use frequency as we are relying upon RFC5545 for these matters.
 <RR> I have 2 problems with this: - RFC5545 is for iCalendar but the use of 
that definition of frequency has leaked into use-cases not requiring iCalendar 
- Terminology section mentions iCalendar (RFC5545) but no mention of frequency. 
Please add it there.    I am not a fan of mixing interval and frequency. But 
I'll leave it to the WG. [Med] Thanks. Added new terms: 
https://github.com/netmod-wg/schedule-yang/pull/52/files     
> - 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.

[Med] We are aware about that guidance however we added "recurrence-" for some 
of the items you mentioned in order to cover cases where, e.g., both period and 
recurrence are used within the same choice. Please see 
https://github.com/netmod-wg/schedule-yang/pull/37 where we made that change.



 <RR> The fact that the recurrence- prefix is used for some leaf nodes to me 
indicates that a recurrence container would be useful.    [Med] Will need to 
think about this one further to see if a surrounding container makes things 
easily consumable for future modules, etc.

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

[Med] This is more about limit. FWIW, "bound" was used here as we leverage RFC 
5545 where we "grabbed" some naming. <RR> I took a look at RFC5545 and it's 
still not clear. The YANG description here says "Modes to bound the recurrence 
rule.", still not clear to me. If not "recurrence-limit", what about 
"recurrence-end", "recurrence-max", may be not ideal but IMO than 
recurrence-bound.    [Med] Changed to « end ». _________________ 
______________________________ ______________________________ 
______________________________ _
Ce message et ses pieces jointes peuvent contenir des informations 
confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce 
message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages 
electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou 
falsifie. Merci.

This message and its attachments may contain confidential or privileged 
information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete 
this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been 
modified, changed or falsified.
Thank you.  
_______________________________________________
netmod mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to