Re: [Netconf] comments on draft-ietf-netconf-subscribed-notifications-12

Martin Bjorklund <mbj@tail-f.com> Fri, 08 June 2018 14:22 UTC

Return-Path: <mbj@tail-f.com>
X-Original-To: netconf@ietfa.amsl.com
Delivered-To: netconf@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id B3119130EB8 for <netconf@ietfa.amsl.com>; Fri, 8 Jun 2018 07:22:36 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.901
X-Spam-Level:
X-Spam-Status: No, score=-1.901 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id RdkylYa7cKEz for <netconf@ietfa.amsl.com>; Fri, 8 Jun 2018 07:22:34 -0700 (PDT)
Received: from mail.tail-f.com (mail.tail-f.com [46.21.102.45]) by ietfa.amsl.com (Postfix) with ESMTP id B7FDC130EB7 for <netconf@ietf.org>; Fri, 8 Jun 2018 07:22:34 -0700 (PDT)
Received: from localhost (h-80-27.A165.priv.bahnhof.se [212.85.80.27]) by mail.tail-f.com (Postfix) with ESMTPSA id EDB271AE027A; Fri, 8 Jun 2018 16:22:33 +0200 (CEST)
Date: Fri, 08 Jun 2018 16:22:33 +0200 (CEST)
Message-Id: <20180608.162233.994500338881044294.mbj@tail-f.com>
To: evoit@cisco.com
Cc: netconf@ietf.org
From: Martin Bjorklund <mbj@tail-f.com>
In-Reply-To: <9f987f8f571e4a499c589f4be02c0407@XCH-RTP-013.cisco.com>
References: <381e3937e0054984812ea69de97c7659@XCH-RTP-013.cisco.com> <20180608.110205.217184993423575402.mbj@tail-f.com> <9f987f8f571e4a499c589f4be02c0407@XCH-RTP-013.cisco.com>
X-Mailer: Mew version 6.7 on Emacs 24.5 / Mule 6.0 (HANACHIRUSATO)
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: <https://mailarchive.ietf.org/arch/msg/netconf/Ne2ets-b2EU_C0Ht0twhw6n2dHQ>
Subject: Re: [Netconf] comments on draft-ietf-netconf-subscribed-notifications-12
X-BeenThere: netconf@ietf.org
X-Mailman-Version: 2.1.26
Precedence: list
List-Id: Network Configuration WG mailing list <netconf.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/netconf>, <mailto:netconf-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/netconf/>
List-Post: <mailto:netconf@ietf.org>
List-Help: <mailto:netconf-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/netconf>, <mailto:netconf-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 08 Jun 2018 14:22:37 -0000

"Eric Voit (evoit)" <evoit@cisco.com> wrote:
> Updated version at:
> https://github.com/netconf-wg/rfc5277bis/blob/master/draft-ietf-netconf-subscribed-notifications-13.txt
> 
> > From: Martin Bjorklund, June 8, 2018 5:02 AM
> > 
> > Hi,
> > 
> > 
> > "Eric Voit (evoit)" <evoit@cisco.com> wrote:
> > > Hi Martin,
> > >
> > > Updated file at:
> > > https://github.com/netconf-wg/rfc5277bis/blob/master/draft-ietf-netcon
> > > f-subscribed-notifications-13.txt
> > >
> > > > From: Martin Bjorklund, June 7, 2018 11:32 AM
> > 
> > [...]
> > 
> > > > o  Section 2.3
> > > >
> > > >   s/"QoS" feature/"qos" feature/  (twice)
> > 
> > You forgot this one.  The feature in the YANG model is "qos", not
> > "QoS".
> 
> Change made.
> 
> > > > o  Section 4
> > > >
> > > >   You have:
> > > >
> > > >           leaf pushed-notifications {
> > > >             type yang:counter64;
> > > >             config false;
> > > >             description
> > > >               "Operational data which provides the number of update
> > > >                notification messages pushed to a receiver.";
> > > >           }
> > > >
> > > >   Is this a left-over from previous versions?  This document doesn't
> > > >   define YANG push, and it doesn't define the term "update
> > > >   notification message".
> > > >
> > > >   Should it simply be notifications-sent, with description "A count of
> > > >   the number of notifications sent to the receiver."?
> > >
> > > Text wasn't left over.  But to make it as clean as possible, I changed
> > > to:
> > >
> > >           leaf count-sent {
> > >             type yang:counter64;
> > 
> > You may want to use yang:zero-based-counter64.
> 
> Change made
>  
> > >             config false;
> > >             description
> > >               "The number of event records sent to the receiver.  The
> > >               count is initialized when a dynamic subscription is
> > >               established, or when a configured subscription
> > >               transitions to the valid state.";
> > >           }
> > >           leaf count-excluded {
> > >             type yang:counter64;
> > 
> > From the description below, you definately want
> > yang:zero-based-counter64.
> 
> Change made
> 
> > >             config false;
> > >             description
> > >               "The number of event records explicitly removed either
> > >               via an event stream filter or an access control filter so
> > >               that they are not passed to a receiver.  This count is
> > >               set to zero each time 'count-sent' is initialized.";
> > >           }
> > 
> > 
> > Ok with this, but I would prefer more descriptive leaf names.
> > event-records-sent and event-records-excluded perhaps.
> 
> Left it as "count-sent" and "count-excluded".  Reason is that the
> "count-sent" can be reused for yang-push notifications without casual
> users needing to read through all the descriptions to understand that
> a "push-update" notification is actually a form of event record.

But the name of the leaf doesn't change the semantics.  The
description says "number of event records", so your casual user still
have to understand that a YANG Push notif is an event record.

BTW, it is not clear from the YANG push document that a YANG push
notif really is an event record.  It uses the term "update record",
and use the term "event record" in just one place.  This term should
be imported from Subscribed-notifications, and used.


/martin


> > > > o  Section 4
> > > >
> > > >     leaf encoding  should have "if-feature configured;"
> > > >
> > > >     (it is dependent on ../transport, which has the if-feature)
> > >
> > > The encoding can be set by RPC, so the issue is with the constraint.
> > > > (E.g., when HTTP is used, JSON and CBOR might be encodings.)
> > >
> > > Maybe the way to address this (as can be seen in the draft) is to
> > > > enhance the constraint to:
> > >
> > > When  ' not(boolean(../transport))  or derived-from(../transport,
> > "sn:configurable-encoding")'
> > 
> > You don't need the boolean(...):
> > 
> >   when 'not(../transport) or derived-from(...)';
> 
> Change made
> 
> Eric
> 
> > /martin
> > 
> > 
> > 
> > > And add an identity error to cover when an attempt is made establish a
> > > subscription with an unsupported encoding:
> > >
> > >   identity encoding-not-supported {
> > >     base establish-subscription-error;
> > >     description
> > >       "Unable to encode notification messages in the desired format.";
> > >   }
> > >
> > >
> > > >     Also, the description for this leaf is in -12:
> > > >
> > > >         "The type of encoding for the subscribed data. If not
> > > >         included as part of the RPC, the encoding MUST be set by the
> > > >         publisher to be the encoding used by this RPC.";
> > > >
> > > >    and in the not-yet-published -13 (from github):
> > > >
> > > >         "The type of encoding for the subscribed data.   If not
> > > >         included, the encoding used will be the default for one
> > > >         encoding expected with a transport.";
> > > >
> > > >    I can't parse from "the encoding used ...".
> > >
> > > Made it:
> > >
> > > "The type of encoding for notification messages.  For a dynamic
> > > subscription,
> > if not included as part of an establish-subscription RPC, the encoding
> > will be
> > populated with the encoding used by that RPC.  For a configured
> > subscription,
> > if not explicitly configured the encoding with be the default encoding
> > for an
> > underlying transport.";
> > >
> > > Thanks,
> > > Eric
> > >
> > > > /martin
> > > >
> > > > _______________________________________________
> > > > Netconf mailing list
> > > > Netconf@ietf.org
> > > > https://www.ietf.org/mailman/listinfo/netconf
> > >
>