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

[Daniele Ceccarelli <[e-mail]>]

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.

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