Re: [alto] Alexey Melnikov's No Objection on draft-ietf-alto-incr-update-sse-20: (with COMMENT)

["Y. Richard Yang" <[e-mail]>]

Thanks for the reviews, Alexey!

Please see inline.

On Wed, Mar 11, 2020 at 12:49 PM Alexey Melnikov via Datatracker <
[e-mail]> wrote:

> Alexey Melnikov has entered the following ballot position for
> draft-ietf-alto-incr-update-sse-20: No Objection
>
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut this
> introductory paragraph, however.)
>
>
> Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
> for more information about IESG DISCUSS and COMMENT positions.
>
>
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-alto-incr-update-sse/
>
>
>
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> Thank you for your document. It was generally a pleasure to read. I have a
> few
> minor things that I would like to discuss before recommending approval of
> the
> document:
>
> 5.3.  ALTO Control Update Message
>
>    description:  a non-normative text providing an explanation for the
>         control event.  When an update stream server stops sending data
>         update messages for a resource, it is RECOMMENDED that the
>         update stream server use the description field to provide
>         details.
>
> I think you should make it more explicit that this is human readable.


> I am not going to insist on using language tags, because I don't think
> this is
> going to work for messages generated by developers for developers.
>

Good suggestion. I can see that some server just sent some, non
human-readable, (server-internal) error code,
which then breaks the intention of the field.

How does the following look:
    description:  a non-normative, human-readable text providing an
explanation for the
        control event.  When an update stream server stops sending data
        update messages for a resource, it is RECOMMENDED that the
        update stream server use the description field to provide
        details. There can be multiple reasons which trigger a "stopped"
event; see above.
        The intention of this field is to provide a human-readable text for
the developer
        and/or the administrator to diagnose potential problems.



>
> 6.7.1.  Event Sequence Requirements
>
>    o  When the ALTO client uses the stream control service to stop
>       updates for one or more resources (Section 7), the ALTO client
>       MUST send a stream control request.  The update stream server MUST
>       send a control update message whose "stopped" field has the
>       substream-ids of all active resources.
>
> "Active" or "stopped"? If the former, then the name of the field is
> misleading.
> If the latter, than the above sentence needs to be corrected.
>

Oops. Typo. Good catch. It should be stopped.


>
> 7.1.  URI
>
>    The ALTO client MUST evaluate a non-absolute control URI (for
>    example, a URI without a host, or with a relative path)
>
> You might want to add a reference to RFC 3986 here, as it explains relevant
> concepts.
>
>
Good pointer. We will add a reference to RFC 3986.



>    in the context of the URI used to create the update stream.
>
> 7.6.  Response
>
>    If the request is valid but the associated update stream has been
>    closed.  The stream control server MUST return an HTTP "404 Not
>    Found".
>
> I think you have 2 sentences where you really wanted to use 1. I.e, this
> should
> read:
>
>    If the request is valid but the associated update stream has been
>    closed than the stream control server MUST return an HTTP "404 Not
>    Found".
>
>
Thanks for catching the "fragmentation". We will use the single sentence
(than -> then).


> With recent IESG recommendations to always use encryption, I recommend you
> use
> https:// instead of http:// URIs in examples.
>
>
Good suggestion. We will switch to use https:// in all examples.


> Media Type registrations should use "[RFCXXXX]" or similar convention
> instead
> of just saying "this document", because media type registrations are cut &
> pasted to IANA website as separate documents.
>

Very thoughtful comment. We will fix and add a note to the RFC editor.

Thanks again for the review!

Richard