Skip to content

Comments

Add a document covering protobuf value encoding.#99

Merged
robshakir merged 2 commits intomasterfrom
pbdoc
Jun 12, 2018
Merged

Add a document covering protobuf value encoding.#99
robshakir merged 2 commits intomasterfrom
pbdoc

Conversation

@robshakir
Copy link
Member

 * (A) rpc/gnmi/protobuf-vals.md
   - Add a document covering the use of protobuf binary values in
     gNMI. This document provides a companion document covering
     the protobuf_val field in TypedValue.

 * (A) rpc/gnmi/protobuf-vals.md
   - Add a document covering the use of protobuf binary values in
     gNMI. This document provides a companion document covering
     the protobuf_val field in TypedValue.
@robshakir robshakir requested review from aashaikh and gcsl June 12, 2018 19:56
Copy link
Contributor

@aashaikh aashaikh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of minor comments

*Implementation Status*: Proposal

## Problem Statement
For some applications the data carried by gNMI is directly described by a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add comma , "For some applications, the data ..."

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

(the concatenated `path` and `prefix` fields) already indicates which message
is serialised within the Notification.

* Results in compatibilities when generated protobufs are within different
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

incompatibilities ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

when generating an augmented version of a standard schema. In this case, the
augmented messages are backwards compatible with the standard messages, since
they are a strict superset of the fields, and hence can be deserialised into
the "standard" message. The differing type in protobuf.Any does not allow
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add backticks to protobuf.Any as above ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

tree ID to a particular container or list element within the YANG schema as
defined by [RFC6020 Section
6.5](https://tools.ietf.org/html/rfc6020#section-6.5). The schema path MUST
NOT be a path to a leaf element. * The `message-name` leaf MUST represent a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stray asterisk ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. vim wrapping issue!

@robshakir robshakir merged commit ca678e1 into master Jun 12, 2018
@robshakir robshakir deleted the pbdoc branch June 12, 2018 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants