Re: [netconf] mbj review of draft-ietf-netconf-notification-capabilities-01

[Martin Bjorklund <[e-mail]>]

Hi,

Pruning...


Balázs Lengyel <[e-mail]> wrote:
> Thanks for the comments, See below. Balazs
> 
> On 2019. 03. 25. 21:56, Martin Bjorklund wrote:
> 
>     Hi,
> 
> Here are my (mostly minor) comments on
> draft-ietf-netconf-notification-capabilities-01

[...]

>     o  General
> 
>   In YP, support for on-change is optional.   There is a feature
>   called "on-change".  I think this document should state explicitly
>   that this module is intended for servers that implement that
>   feature.
> 
> BALAZS: OK, but some other capacity related proerties are still valid.

What other capacity related properties do you mean?

>     o  Section 3.2
> 
>   You have:
> 
>          units msec;
> 
>          units centiseconds;
> 
>   I think you probably should do s/msec/milliseconds/
> 
> BALAZS: OK, probably change all to centiseconds. Is there a general
> rule not to use the official abbreviations?

What is the official abbrevation?  According to wikipedia, the SI name
is "millisecond" and "centisecond", and the corresponding symbols are
"ms" and "cs".  I briefly scanned the SI spec, but didn't really come
to any conclusion...

My point was just that it is probably best to be consistent and not
use an abbreviation in one place and the full word in the other.

>     o  Section 3.2
> 
>   minimum-dampening-period is optional.  It should be stated what it
>   means if this leaf is not present.   (or should the default be 0?)
> 
> BALAZS: If not present it means that the system does not tell you what
> it is. No special meaning. I do not see a reasonable minimum value
> (eventhough 0 would look strange). IMHO it is the responsibility of
> the server to provide a meaningful value. If it says zero, that's
> obviously crazy, just ignore it. I don't thing we need to have rules,
> like: do not provide stupid state data.

Note that 0 is the default in YP, so I don't know why you think it is
crazy.

I suggest you add default "0" to this.

>     o  Section 3.2
> 
>   The choice update-period is not mandatory, and doesn't have a
>   default.  What does it mean if this is not reported?
> 
> BALAZS: If not present it means that the system does not tell you what
> it is. No special meaning.

I think this should be clarified in the description.

>     o  Section 3.2
> 
>   The leaf max-objects-per-update is not mandatory.  What does it mean
>   if this is not reported?  It can have the value 0.  What does that
>   mean?
> 
> BALAZS: If not present it means that the system does not tell you what
> it is. No special meaning.
>
> I do not see a reasonable minimum value
> (eventhough 0 would look strange). IMHO it is the responsibility of
> the server to provide a meaningful value. If it says zero, that's
> obviously crazy, just ignore it.

I suggest you add a range "1..max" to the type.

>       Perhaps make a union of uint32 0..max and the enum "infinite" and
>   make that the default?  (of course "infinite" is not really
>   correct...)
> 
> BALAZS: IMHO infinite is unreasonable. If the server doesn't know the
> real limit, it should just not provide a value.

Ok, makes sense.  I think this should be clarified in the description.


/martin