Re: [CCAMP] YANG issue in draft-ietf-ccamp-alarm-module

[Martin Bjorklund <[e-mail]>]

Daniele Ceccarelli <[e-mail]> wrote:
> Thanks a lot Martin for finding the issue and letting the WG know about it. 
> 
> > I.e., change it to an enumeration leaf with a default value, and add an
> additional leaf to set the severity level when the enum "severity-level" is
> set.
> 
> I don't see this as a major change. Please update the draft, we won't need
> another round of review.

The draft (-08) is now posted with these fixes.


/martin



> 
> Thanks
> 
> Daniele  
> 
> -----Original Message-----
> From: CCAMP <[e-mail]> On Behalf Of Martin Bjorklund
> Sent: den 22 mars 2019 13:43
> To: [e-mail]
> Subject: [CCAMP] YANG issue in draft-ietf-ccamp-alarm-module
> 
> Hi,
> 
> When we fixed the review comments for this draft, I found an issue with the
> current YANG model.  Currently we have:
> 
> 
>          choice notify-status-changes {
>            description
>              "This leaf controls the notifications sent for alarm status
>               updates.  There are three options:
> 
>               [...]";
> 
>            leaf notify-all-state-changes {
>              type empty;
>              description
>                "Send notifications for all status changes.";
>            }
>            leaf notify-raise-and-clear {
>              type empty;
>              description
>                "Send notifications only for raise, clear, and re-raise.
>                 Notifications for severity level changes or alarm text
>                 changes are not sent.";
>            }
>            leaf notify-severity-level {
>              type severity;
>              description
>                "Only send notifications for alarm state changes crossing
>                 the specified level.  Always send clear notifications.";
>            }
>          }
> 
> 
> First of all, the description refers to the choice as a leaf; the main text
> has the same issue.
> 
> But worse is that it isn't described what happens if no leafs in this choice
> are configured.  What is the default?
> 
> Since the leafs are of type empty we can't just add a default statement to
> the choice.
> 
> So we suggest the following updated model to fix this issue:
> 
>       leaf notify-status-changes {
>         type enumeration {
>           enum all-state-changes {
>             description
>               "Send notifications for all status changes.";
>           }
>           enum raise-and-clear {
>             description
>               "Send notifications only for raise, clear, and
>                re-raise.  Notifications for severity level changes or
>                alarm text changes are not sent.";
>           }
>           enum severity-level {
>             description
>               "Only send notifications for alarm state changes
>                crossing the level specified in
>                'notify-severity-level'.  Always send clear
>                notifications.";
>           }
>         }
>         must '. != "severity-level" or ../notify-severity-level' {
>           description
>             "When notify-status-changes is 'severity-level', a value
>              must be given for notify-severity-level.";
>         }
>         default "all-state-changes";
>         description
>           "This leaf controls the notifications sent for alarm status
>            updates.  There are three options:
> 
>            [...]";
>       }
>       leaf notify-severity-level {
>         when '../notify-status-changes = "severity-level"';
>         type severity;
>         description
>           "Only send notifications for alarm state changes crossing
>            the specified level.  Always send clear notifications.";
>       }
> 
> 
> I.e., change it to an enumeration leaf with a default value, and add an
> additional leaf to set the severity level when the enum "severity-level" is
> set.
> 
> Note that it doesn't change any major concepts; it is a model change to
> better describe the semantics.
> 
> Unless we hear any objections, we'll make this change in the updated draft
> that will be sent to the IESG.
> 
> 
> 
> /martin
> 
> _______________________________________________
> CCAMP mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/ccamp