Hi,

Thank you for this review.  Comments inline.

Ladislav Lhotka <lho...@nic.cz> wrote:
> Hi,
> 
> here is my review of the rfc7895bis document:
> 
> *** General comments
>     - This revision is a significant improvement necessary for
>       supporting NMDA. However, it is also flexible enough in that it
>       allows for implementing the NMDA rules in an effective way but
>       doesn't preclude the use of other datastores and datastore
>       architectures.
> 
>     - Another benefit is that the new YANG library schema can be
>       easily integrated with schema mount information.
> 
> *** Specific comments
> 
> **** Sec. 1 - backward compatibility
> 
>      Given that the old YANG library schema is carried over as a
>      deprecated subtree, how can a server implementor actually cater
>      for backward compatibility of e.g. RESTCONF clients supporting
>      only RFC 8040?  Could the same YANG modules that comprise NMDA
>      datastore schemas be advertized also via the old YANG library? Or
>      is it necessary to also have pre-NMDA versions of all modules?
> 
>      Some more explanation might be useful here.

The text says:

  The recommended approach to populate /modules-state is to
  report the schema for YANG modules that are configurable via
  conventional datastores and for which config false data nodes are
  returned via a NETCONF <get> operation, or equivalent.

Do you think that more guidance is needed?

My opinion is that a server that wasnt to be backwards compatible
probably advertise exactly what it used to advertise (in
/modules-state), even if new NMDA-compatible models are added and
advertised in /yang-library for new clients.

In general it is of course not possible to advertise everything that
is availabile in /yang-library also in /modules-state - if this was
possible we wouldn't have done YLbis.

I don't think this document should provide recommendations on whether
to implement pre-NMDA versions or not; this will be up to each server
implementor to decide.

> **** Sec. 1 - YANG library stability
> 
>      The text basically says that the YANG library information can
>      change at any time. This has been recently discussed but I
>      haven't seen any conclusion yet. I understand it is difficult to
>      enumerate all the situations when this information can change,
>      but it should also be emphasized that YL info is not just another
>      subtree of state data and that it should not change haphazardly.

I agree, but I think that YANG library's job is to report what the
server implements.  If the server dynamically changes its set of
loaded modules, then YL should adapt.

I welcome more discussion on this topic, but I don't think it has to
be documented in this draft.

>      It is like with database schemas, REST APIs and the like. Of
>      course, these can change as well, but everybody has to understand
>      that doing so means transition problems, broken clients etc.
> 
>      For this reason, it might be useful to set YL and schema mount
>      data aside and call them metadata or schema information - even if
>      we continue modelling them with YANG.

Do you have some concrete proposal for where to introduce this term?

> **** Sec. 3 - semantic versioning
> 
>      Some placeholders for future inclusion of semantic versions in
>      YANG library information might be useful. To this end, I would
>      suggest to introduce a new choice node and make "revision" (or,
>      even better, "revision-date") its only case. This way, other
>      versioning schemes such as semver could be easily added via
>      augmentation.

When revision is used as a list key, we can't have a choice.

And when it is not a key, it is an optional leaf, so adding a 'semver'
optional leaf in the future is also ok.

The proposal I have seen adds semver as an addition to revision, so
using a choice would not be correct in this case.


> **** Sec. 4 - checksum
> 
>      I think it would be very useful (even if not immediately) to
>      standardize the procedure for computing the checksum. What I
>      envision are systems that construct and process YANG schemas
>      (such as the YANG Catalog). They could benefit from having a
>      universal hash string as a characteristic of any particular
>      schema. Just consider how useful the universal hashes are e.g. in
>      git.

Ok.  It would be interesting to see such a scheme.  But I agree it is
not needed immediately for this document.

> **** Nits
> 
>      - sec 1. paragraph 2: s/informaton/information/

Fixed.


/martin


> 
> Lada
> 
> -- 
> Ladislav Lhotka
> Head, CZ.NIC Labs
> PGP Key ID: 0xB8F92B08A9F76C67
> 
> _______________________________________________
> netmod mailing list
> netmod@ietf.org
> https://www.ietf.org/mailman/listinfo/netmod
> 
>

_______________________________________________
netmod mailing list
netmod@ietf.org
https://www.ietf.org/mailman/listinfo/netmod

Reply via email to