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

[Alexey Melnikov <[e-mail]>]

Hi Richard,

On Wed, Mar 11, 2020, at 9:12 PM, Y. Richard Yang wrote:
> 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.
This is better, thank you.

>> 
>> 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.
Great.
>> 
>> 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).
Yes, indeed!

> 
>> 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!
You are very welcome.

Best Regards,
Alexey