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