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

From: "Murray, Rob (Rob)" <[email-masked]>
Date: Sun, 6 Dec 2015 18:48:53 +0000

Hi all - I've just submitted version "-10" of the triggers doc to address Barry's comments and update refs.

Barry, thank you for all the comments and suggested fixes. Responses below, but maybe worth noting that refs to RFCs 2818 and 6707 are still informative in this revision - they're informational RFCs so "idnits" doesn't like normative refs to them. (But I'll make them normative in the next revision if you think that's the right thing to do.) I hope the section about "PatternMatch" makes more sense now, but let me know if not.

Cheers,
Rob.

On 23/11/2015, 03:28, "[email-masked] on behalf of Barry Leiba" <[email-masked] on behalf of [email-masked]> wrote:

>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

Done.

>
>-- 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.

Done.

>
>-- 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

Done.

>   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"?)

Done.

>-- 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

Done.

>-- 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....

Done.

>-- 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?

I can't remember where it came from, but it's gone now.

>-- 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

Done.

>      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?

The pattern match is always against a URI, but that wasn't stated. The "query string" referred to the query part of the URI.

I've changed the first paragraph of the section from:

    A Pattern Match consists of a string pattern to match, and
    flags describing the type of match.

To:

    A Pattern Match consists of a string pattern to match
    against a URI, and flags describing the type of match.

The description for "pattern" from:

    Description: A pattern for string matching.

To:

    Description: A pattern for URI matching.

And the description of match-query-string from:

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

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

    Mandatory: No, default is not to include the query string in
    the pattern match.

To:

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

    Value: One of the JSON values 'true' (the full URI including
    the query part should be compared against the given pattern),
    or 'false' (the query part of the URI should be dropped before
    comparison with the given pattern).

    Mandatory: No, default is 'false', the query part of the URI
    should be dropped before comparison with the given pattern.

Does that address the problem?

>-- 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

Done.

>-- 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.

The examples are all generated by a simulator of the interface that runs under the Python Twisted web server - so it generates the Content-Length headers for me.

But Python's JSON encoder / pretty-printer was putting whitespace on the end of some lines, which was invisible in the draft. So I've stripped that whitespace before putting it in the response, and re-generated the examples.

The JSON in the message body doesn't have (or need) CRLF line-endings, whitespace isn't significant in JSON. HTTP headers don't contribute to the Content-Length.

Let me know if you still see anything dodgy.

>-- 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

"idnits" doesn't like normative references to informational RFCs, which makes sense I guess ... so I've left them alone.

>- 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.

Text from section 5.3 moved to a non-normative appendix, as discussed on the list. (So ref not made normative.)

>--
>Barry

---

From: Barry Leiba <[email-masked]>
Date: Mon, 7 Dec 2015 10:12:42 -0500

Hi, Rob, and thanks for addressing my comments.  Unnecessary stuff trimmed here:

> The pattern match is always against a URI, but that wasn't stated.
> The "query string" referred to the query part of the URI.

Ahhhh, yes, I see.  Your suggested change is good; thanks.

> The examples are all generated by a simulator of the interface that
> runs under the Python Twisted web server - so it generates the
> Content-Length headers for me.
>
> But Python's JSON encoder / pretty-printer was putting whitespace
> on the end of some lines, which was invisible in the draft. So I've
> stripped that whitespace before putting it in the response, and
> re-generated the examples.

Great thanks.

> The JSON in the message body doesn't have (or need) CRLF
> line-endings, whitespace isn't significant in JSON.

Right, but if CRLFs are included, their presence counts in the content
length.  And there are line breaks included in the draft, so a reader
would assume those are sent on the wire in the HTTP transaction.

But I'm OK either way.  If you're satisfied that the content-length
values are correct, I'm not going to push on it.

> "idnits" doesn't like normative references to informational RFCs, which
> makes sense I guess ... so I've left them alone.

IDnits is a tool, not a policy, and it's OK to have IDnits call
something out, if that "something" is the right thing to do.  Here,
it's the right thing to do, and if we ultimately make them normative
then I have to call that out during last call (that's what IDnits is
reminding us of).

So please make them normative now, and I'll make sure the last call
notice lists them.

Barry

---

From: "Murray, Rob (Rob)" <[email-masked]>
Date: Mon, 7 Dec 2015 15:23:41 +0000

Thanks Barry - I've normativeified those refs and published "-11".

Rob.