Subject: Re: [Gen-art] Genart last call review of draft-ietf-core-senml-etch-05

From: Alissa Cooper <[email-masked]>
Date: Wed, 4 Sep 2019 15:41:56 -0400

Robert, thanks for your review. I pointed to it in my No Objection ballot.

Best,
Alissa

> On Aug 29, 2019, at 10:38 AM, Robert Sparks via Datatracker <[email-masked]> wrote:
> 
> Reviewer: Robert Sparks
> Review result: Ready with Nits
> 
> I am the assigned Gen-ART reviewer for this draft. The General Area
> Review Team (Gen-ART) reviews all IETF documents being processed
> by the IESG for the IETF Chair.  Please treat these comments just
> like any other last call comments.
> 
> For more information, please see the FAQ at
> 
> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
> 
> Document: draft-ietf-core-senml-etch-05
> Reviewer: Robert Sparks
> Review Date: 2019-08-29
> IETF LC End Date: 2019-09-02
> IESG Telechat date: 2019-09-05
> 
> Summary: Ready for publication as a Proposed Standard RFC, but with nits to
> consider before publication
> 
> Nits:
> 
> Since the string "-etch-" is in the media type, it might be nice to say in the
> document where it came from.
> 
> I think the text in the interoperability considerations sections of the
> registrations could be improved. You mean to talk about unrecognized keys, not
> unrecognized key-value pairs. I also think the body of the RFC should have a
> very short extensibility section that explicitly says you're doing a similar
> thing as 8424 section 4.4 and point to that section.
> 
> I am a little uncomfortable with the "Fragment Identification" section (4) of
> this document - it feels like a "do what we mean" statement. I don't have text
> to suggest. It may well be that it will be dead-obvious to an implementer what
> to do, but it makes me uneasy.
> 
> 
> _______________________________________________
> Gen-art mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/gen-art

---

From: Ari Keränen <[email-masked]>
Date: Sat, 14 Sep 2019 16:03:20 +0000

Hi Alissa, Robert & Warren,

Thank you for your reviews! You all had concerns on the Fragment ID part of the SenML etch draft (quotes below). I'll try to address those concerns in this PR:
https://github.com/core-wg/senml-etch/pull/10

The suggested new Fragment ID section text would be:

   Fragment identification for Records of Fetch and Patch Packs uses the
   same mechanism as SenML JSON/CBOR fragment identification (see
   Section 9 of [RFC8428]), i.e., "rec" scheme followed by a comma-
   separated list of Record positions or range(s) of Records.  For
   example, to select the 3rd and 5th Record of a Fetch or Patch Pack, a
   fragment identifier "rec=3,5" can be used in the URI of the Fetch or
   Patch Pack resource.

More details below.

> On 29 Aug 2019, at 17.38, Robert Sparks via Datatracker <[email-masked]> wrote:
[...]
> to do, but it makes me uneasy.

I was hoping it to be obvious since it is exactly the same mechanism as for regular SenML; just applied to another media type with the same structure. I'm a bit hesitant to add more normative text since that would be just repeating what's already said in the SenML RFC. Instead I'd suggest a short summary of the feature and an example (see above). 

Warren had a good observation that is wasn't clear to what Pack the ID applies to so I clarified that (see below).

> On 3 Sep 2019, at 22.23, Warren Kumari via Datatracker <[email-masked]> wrote:
[...]
> Question:
> 1: The text in Section 4 feels quite hand-wavy / terse, and I don't think gives
> sufficient guidance to actually use this. e.g: What takes precedence? Do I
> refer to a specific record (using fragment identification) and then apply the
> FETCH / PATCH to that? Or do I use fragment identification to refer to records
> what have been PATCHed? As might be clear from the above, I'm not a CoRE
> person, so I'll be happy to accept "Your question makes no sense, this will be
> blindingly obvious to anyone who's actually implementing this...." :-)

Actually this is a mechanism to refer to Records of the Patch/Fetch Pack, not in the resulting (regular SenML) Pack. I suggest clarifying this in the text with:

OLD: Fragment identification is supported by analogously applying [...]
NEW: Fragment identification for Records of Fetch and Patch Packs uses [...]

I hope this solves your concerns.

Thanks,
Ari