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 > <[email protected]> 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
_______________________________________________ alto mailing list [email protected] https://www.ietf.org/mailman/listinfo/alto
