Hi Jean, Sorry for coming back, but I'm still not 100% sure that we are in sync. Please see further comment and clarification inline ...
> -----Original Message----- > From: Jean Quilbeuf <jean.quilb...@huawei.com> > Sent: 06 November 2022 15:47 > To: Rob Wilton (rwilton) <rwil...@cisco.com>; opsawg@ietf.org; draft-ietf- > opsawg-service-assurance-yang....@ietf.org > Subject: RE: AD review of draft-ietf-opsawg-service-assurance-yang-07 > > Hi Rob, > Thanks for the answers. > > I think we agree on the final goal and as I understand it is what we have in - > 08, so let’s go with that version. More details below. > > Thanks, > Jean > > > -----Original Message----- > > From: Rob Wilton (rwilton) [mailto:rwil...@cisco.com] > > Sent: Sunday 6 November 2022 15:22 > > To: Jean Quilbeuf <jean.quilb...@huawei.com>; opsawg@ietf.org; draft- > > ietf-opsawg-service-assurance-yang....@ietf.org > > Subject: RE: AD review of draft-ietf-opsawg-service-assurance-yang-07 > > > > Hi Jean, > > > > Just one further question inline ... > > > > > -----Original Message----- > > > From: Jean Quilbeuf <jean.quilb...@huawei.com> > > > Sent: 19 October 2022 01:10 > > > To: Rob Wilton (rwilton) <rwil...@cisco.com>; opsawg@ietf.org; > > > draft-ietf- opsawg-service-assurance-yang....@ietf.org > > > Subject: RE: AD review of draft-ietf-opsawg-service-assurance-yang-07 > > > > > > Hi Rob, > > > Thank you very much for you detailed review. > > > > > > The latest version should address most of the comments. The diff is here: > > > https://www.ietf.org/rfcdiff?url2=draft-ietf-opsawg-service-assurance- > > > yang- > > > 08 > > > > > > Answers to some unanswered or added details are inline below. > > > > > > Thanks Again > > > Best, > > > Jean > > > > > > > > > > -----Original Message----- > > > > From: Rob Wilton (rwilton) [mailto:rwil...@cisco.com] > > > > Sent: Monday 17 October 2022 13:06 > > > > To: opsawg@ietf.org; > > > > draft-ietf-opsawg-service-assurance-yang....@ietf.org > > > > Subject: AD review of draft-ietf-opsawg-service-assurance-yang-07 > > > > > > > > Dear authors, > > > > > > > > And here is my corresponding AD review of draft-ietf-opsawg-service- > > > > assurance-yang-07. Again, I found the document to be well-written, > > > > with mostly minor comments/suggestions, bar one question about the > > > symptom > > > > reference. > > > > > > > > > > > > (12) p 6, sec 3.2. Tree View > > > > > > > > The "health-score" contains a value normally between 0 and 100 > > > > indicating how healthy the subservice is. The special value -1 can > > > > be used to specify that no value could be computed for that health- > > > > score, for instance if some metric needed for that computation could > > > > not be collected. > > > > > > > > Because this is an enum, this would often/normally be encoded as the > > > string > > > > "missing" rather than as -1. > > > > > > I am hoping that YANG-aware deserialization will replace the value -1 > > > by the enum name, i.e. "missing" when targeting a human. The reason to > > > use a number is for homogeneity with the health score, thinking of > > > time series databases which fail if a time series changes type (int -> > > > string for instance) over time. > > > > This is okay, but it does mean that the deserialization code will need to > > potentially spot that it may be the string value "missing" and then decide > to > > convert that a reserved integer, presumably -1. E.g., it feels like the > > union > > type of integer + string just means that you probably need slightly more > code > > when streaming it into a time series database? > > > > Given that this leaf is marked as being optional in the schema, then it > could > > just be represented as the range 0 to 100, and if the server doesn't know > the > > value then it doesn't provide any value at all. > > Or alternatively, it could be modelled as a (perhaps mandatory true) leaf, > > with the range -1 to 100, with a default value of -1, and an explanation in > the > > description that the value -1 is used to indicate that no health score is > being > > provided. > > > > But there are pretty minor comments on the encoding. Please let me know > > if you would like to change this and post a -09, or if you would like me to > last > > call -08 as it is? > > In the end, the current version is union of range 0-100 with value -1 (so > morally equivalent to a range -1 to 100), with the explicit information that - > 1 means "missing": > > leaf health-score { > type union { > type uint8 { > range "0 .. 100"; > } > type enumeration { > enum missing { > value -1; > description > "Explictly represent the fact that the health score is > missing. This could be used when metrics crucial to > establish the health score are not collected anymore."; > } > } > }. > > I think the only difference with what you’re proposing is that health score > is > not mandatory, which I think shouldn’t be as there are use cases where we > might just be interested in the graph structure. The difference will be what values you see on the wire (e.g., in JSON, XML, or CBOR) encoding. In what I propose, on the wire, you will see a numerical value between -1 and 100, so if can easily be written directly into a database that is expecting a numeric value. With the way that you have encoded it, you will either see the UTF-8 string "missing" or a numerical value between 0 and 100. Even CBOR that uses the numerical value for enums doesn’t use the numeric value as part of a union. Hence, if you are writing code to decode a YANG stream and write the value into a database that expects to only store a numerical value for the health score, then the decoder code will need to covert the string "missing" into whatever value the client choses to represent a missing value (e.g., -1). Specifically, YANG does not use the enumeration values on the wire encoding, it uses the string names instead. So, I was really querying what the extra complexity is gaining you ... But if you are still sure that you want this a union, then I can progress -08. Regards, Rob > > > > Regards, > > Rob > > > > > > > > > > > > > > > > Thanks, > > > > Rob _______________________________________________ OPSAWG mailing list OPSAWG@ietf.org https://www.ietf.org/mailman/listinfo/opsawg
