Subject: [CDNi] AD review of draft-ietf-cdni-control-triggers-09

From: Barry Leiba <[email-masked]>
Date: Sun, 22 Nov 2015 22:28:18 -0500

My apologies for the long delay in handling this publication request:
I've had a month straight of travel, including a little personal
vacation at the beginning and end.  I'm back and running full steam,
and reviewing the backlog of publication requests from several working
groups.  Thanks for the patience.

I have a number of comments below.  Almost all of them are minor
editorial things that wouldn't delay last call, but because there are
quite a few and because a couple of them are things I'm questioning
(the comment on "match-query-string" and the one about the
references), I'd like to get the list resolved and a new I-D posted
before I request last call.  None of it's a big deal, and we should be
able to take care of it quickly.

Of course, if you have any questions about any of my comments, or
disagree with any of them, please discuss them with me.

Section 2 starts off using "uCDN" and "dCDN", which are not defined in
6707.  You do mention 7336 (where they are defined in a sort of "by
the way" manner) in the Introduction, but I'm not sure readers would
know to look there for those terms.  I suggest this change to Section
1.1, echoing what 7336 does (it's a pity that they're not in 6707):

OLD
   This document reuses the terminology defined in [RFC6707].

NEW
   This document reuses the terminology defined in [RFC6707], and
   uses "uCDN" and "dCDN" as shorthand for "Upstream CDN" and
   "Downstream CDN", respectively.

END

-- Section 2 --

   There are two types of CI/T Command, CI/T Trigger Commands and CI/T
   Cancel Commands.

The comma isn't strong enough here; it should be a colon (":").

Also, throughout the document, please check for consistent
capitalization of those three terms.  I noticed at least one instance
of "CI/T cancel command" in Section 4.3, and there might be others.

-- Section 4.1 --

   The dCDN validates the CI/T Command, if it is malformed or the uCDN
   does not have sufficient access rights it MUST either respond with an
   appropriate 4xx HTTP error code and a Trigger Status Resource MUST
   NOT be created on the dCDN, or create a 'failed' Trigger Status
   Resource containing an appropriate error description.

This starts with a comma splice and goes on to be oddly worded.  May I
suggest this?:

NEW
   The dCDN validates the CI/T Command.  If the command is malformed or
   the uCDN does not have sufficient access rights, the dCDN MUST either
   respond with an appropriate 4xx HTTP error code and not create a
   Trigger Status Resource, or create a 'failed' Trigger Status
   Resource containing an appropriate error description.
END

   The HTTP response SHOULD
   include the content of the newly created Trigger Status Resource,
   this is recommended particularly in cases where the CI/T Trigger
   Command has completed immediately.

I don't like the "recommended particularly" along with the "SHOULD" --
remember that "SHOULD" and "RECOMMENDED" are synonymous (and there's
also a comma splice here).  I suggest this:

NEW
   The HTTP response SHOULD
   include the content of the newly created Trigger Status Resource.
   This is particularly important in cases where the CI/T Trigger
   Command has completed immediately.
END

(If "important" isn't quite right, maybe "significant"?)

-- Section 4.2 --

   To check for change in status of a Trigger Status Resource or
   collection of Trigger Status Resources without re-fetching the whole
   Resource or Collection, Entity Tags SHOULD be included by the dCDN
   for the uCDN to use as cache validators, as defined in [RFC7232].

The dCDN doesn't include those tags to check for change in status.  It
includes them to allow the uCDN to check for change in status.

NEW
   To allow the uCDN to check for change in status of a Trigger Status
   Resource or collection of Trigger Status Resources without
   re-fetching the whole Resource or Collection, the dCDN SHOULD
   include Entity Tags for the uCDN to use as cache validators, as
   defined in [RFC7232].
END

-- Section 4.3 --

   Cancellation of a CI/T Trigger Command is optional-to-implement in
   the dCDN.

I was initially uncertain about this: it could be read two ways (it
could be read to mean that the dCDN doesn't have to implement the
Cancel Command at all), so it's probably best to be clear (even though
one can get that from the next paragraph).

NEW
   The dCDN is required to accept and respond to the CI/T Cancel
   Command, but the actual cancellation of a CI/T Trigger Command is
   optional-to-implement.
END

Or something like that....

-- Section 4.8 --
The section starts with "therefore".  What's the antecedent of
"therefore"?  Is that a vestige of something that used to be there?
Should "therefore" be removed, or should something be re-worded?

-- Section 5.2.4 --

      Name: case-sensitive

         Description: Flag indicating whether or not case-sensitive
         matching should be used.

         Value: One of the JSON values 'true' or 'false'.

         Mandatory: No, default is case-insensitive match.

I think this is properly specified as it is, so you can ignore this
comment if you prefer, but I'd rather see the descriptions whack the
reader in the face, rather than make anyone guess what "true" or
"false" correspond to:

NEW
      Name: case-sensitive

         Description: Flag indicating whether or not case-sensitive
         matching should be used.

         Value: One of the JSON values 'true' (the matching is case-
         sensitive) or 'false' (the matching is case-insensitive).

         Mandatory: No, default is 'false', a case-insensitive match.
END

      Name: match-query-string

         Description: Flag indicating whether or not the query string
         should be included in the pattern match.

I don't understand what this means; sorry.  Can you explain?

-- Section 5.2.5 --

   A JSON number, seconds since the UNIX epoch.

Because you don't have a reference cited here, it'd be good to say
what the UNIX epoch is.  Even with a reference, it's easy to say it,
and it avoids making the reader look it up:

NEW
   A JSON number, seconds since the UNIX epoch, 00:00:00 UTC
   on 1 January 1970.
END

-- Section 6 --
Thanks for the comprehensive examples; I think that's very useful.

I'm not sure how much it matters (I'm sure some picky person will
submit errata reports), but I don't get the same content-length values
that you do on any of the examples.  Please be sure the content-length
values match the data as indented and with all displayed whitespace,
with assumption of {CR,LF} at the end of each line.

-- References --
I think three references need to be moved from informative to normative:
- RFC 2818 is the subject of a MUST in Section 8.1
- RFC 6707 provides necessary terminology
- draft-greevenbosch-appsawg-cbor-cddl defines the formal language
used in Section 5.3

If you disagree, please tell me why.

Of course, that means that draft-greevenbosch-appsawg-cbor-cddl will
block publication of this document until it's ready.

--
Barry

---

From: "Francois Le Faucheur (flefauch)" <[email-masked]>
Date: Mon, 23 Nov 2015 10:29:43 +0000

Barry, authors,

> block publication of this document until it's ready.

It seems undesirable to create a dependency:
	* whose timing and potentially even outcome (given the document is not even yet a working group document) is out of our control and 
	* that is not strictly necessary (my understanding is that draft-greevenbosch-appsawg-cbor-cddl is referenced and used simply to facilitate description). 

Would it be possible to remove the dependency somehow?
For example, perhaps cdni-control-trigger could redefine the subset of notational conventions needed (and simply reference draft-greevenbosch-appsawg-cbor-cddl for Information as backgrounds on what these conventions are inspired from ?

Cheers

Francois

> --
> Barry
> 
> _______________________________________________
> CDNi mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/cdni

---

From: "Murray, Rob (Rob)" <[email-masked]>
Date: Mon, 23 Nov 2015 12:12:10 +0000

>For example, perhaps cdni-control-trigger could redefine the subset of notational conventions needed (and simply reference draft-greevenbosch-appsawg-cbor-cddl for Information as backgrounds on what…

We could just drop the "cddl" formalisation? I put it in because it might be useful to someone and Carsten had already done the work as part of his review, to cross-check that the description of the JSON structures is consistent. It's not essential to the definition of the interface.

(Thank you for all the comments Barry, I'll take a look through and respond soon as I can.)

Rob.

---

From: Ben Niven-Jenkins <[email-masked]>
Date: Mon, 23 Nov 2015 12:13:47 +0000

Or if we wanted to keep it we could move it to a non-normative appendix? Ben

> On 23 Nov 2015, at 12:12, Murray, Rob (Rob) <[email-masked]> wrote:
> 
> 
> 
> 
> 
> 
> On 23/11/2015, 10:29, "CDNi on behalf of Francois Le Faucheur (flefauch)" <[email-masked] <mailto:[email-masked]> on behalf of [email-masked] <mailto:[email-masked]>> wrote:
> 
>> Barry, authors,
>> 
>> 
>>> 
>>> -- References --
> [email-masked] <mailto:[email-masked]>
> https://www.ietf.org/mailman/listinfo/cdni <https://www.ietf.org/mailman/listinfo/cdni>

---

From: Barry Leiba <[email-masked]>
Date: Mon, 23 Nov 2015 17:16:38 -0500

> Or if we wanted to keep it we could move it to a non-normative appendix? Ben

I think that's the best approach.  By putting it in an appendix and
making it clear that it's provided for information, you can leave the
reference as informative.  That works well for me.

Barry