Hi Jensen, Thanks for your answers, please see mine inline, Thanks, Sabine
From: Jensen Zhang [mailto:[email protected]] Sent: Thursday, December 07, 2017 3:43 AM To: Randriamasy, Sabine (Nokia - FR/Paris-Saclay) <[email protected]> Cc: [email protected]; IETF ALTO <[email protected]> Subject: Re: Update review of draft-ietf-alto-cost-calendar-02 Hi Sabine, It is good to know the revision is in progress. Please see my comments inline. Best, Jensen On Thu, Dec 7, 2017 at 2:18 AM Randriamasy, Sabine (Nokia - FR/Paris-Saclay) <[email protected]<mailto:[email protected]>> wrote: Hi Jensen, Indeed, the revison of the drafts is in progress and thanks a lot for your comments that will be considered for the next version. Please see my feedback inline. In the new text proposals, text in ++blabla++ format means "blabla" is added to the initial text. Thanks, Sabine ________________________________ De : Jensen Zhang <[email protected]<mailto:[email protected]>> Envoyé : samedi 2 décembre 2017 09:04 À : Randriamasy, Sabine (Nokia - FR/Paris-Saclay); [email protected]<mailto:[email protected]> Cc : IETF ALTO Objet : Update review of draft-ietf-alto-cost-calendar-02 Hi Sabine and other authors, How are you? Since the last review from Dawn (https://mailarchive.ietf.org/arch/msg/alto/ZSH5Z1ujBvh4YjQxZLwjPNocXRM/?qid=4922c9c845f02b2bd9c02cc751c74c87), I have not found the draft was updated yet. From the agreement in IETF 99, there are a lot of text harmonization work. I assume the revision is in progress. I just append some technical review for the current version below. Hopefully, it will be helpful. === Section 3.1., paragraph 3: > A member "calendar-attributes" MUST appear only once for each > applicable cost type name of a resource entry. If "calendar- > attributes" are specified several times for a same "cost-type-name" > in the capabilities of a resource entry, the ALTO client SHOULD > ignore any calendar capabilities on this "cost-type-name" for this > resource entry. I think it will be much better to adopt the finest granularity than just ignore all, if there are more than one "calendar-attributes" object for the same "cost-type-name". => OK, the text may be too careful and we may allow a client to consider the first set of attributes and ignore the next ones. The sentence would then become: "... the ALTO client SHOULD ++ only consider the first occurrence of "calendar-attributes and++ ignore any ++additional++ calendar capabilities ..." (Jensen) The modification looks fine for me. Section 3.1., paragraph 9: > * is the duration of an ALTO calendar time interval, expressed as > a time unit appended to the number of these units. The time > unit, ranges from "second" to "year". The number is encoded > with an integer. Example values are: "5 minute" , "2 hour", > meaning that each calendar value applies on a time interval > that lasts respectively 5 minutes and 2 hours. I prefer to use another field (i.e. "time-interval-unit") to express the time unit and make "time-interval-size" a simple JSONNumber. Because it can simplify the data validation. In this way, the server and client only need to check the data type (number and enumeration) without validating the data content. ==> the motivation here was mainly to lighten the data volume by using only one fieldand make sure that a change in either units and number of units will not be missed by the client. Do you mean that the parsing of the value of the current "time-interval-size" is longer and more complex? (Jensen) I just concern the potential error from the different implemetations, when introducing the extra syntax in the data content. For example, if we follow the current definition of "time-interval-size", different servers may implement the description "a time unit appended to the number of these units" in different ways: - One server may specify a regex like /^\d+\w*(second|minute|hour|month|year)$/. - But another server may enforce the expression as a number with **exact one space** with a unit. [[SR]] we may specify the format of "time-interval-size" just as constraint format is specified in section 11.3.2.3 of RFC 7285. Writing for example: a "time-interval-size" value contains 2 entities separated by exactly one whitespace: a JSONNumber and a string taking values in {second, minute, hour, month, year}. Example values are: “1 minute”, “5 month”. - And some other server may allow "week" as a unit. [[SR]] Indeed, week can be added as an alternative to “7 day”. - ... But these information cannot be provided by the capabilities in the IRD entry. So the client may assume the server implementation in some inconsistent way and conduct some unexpected errors. I don't know how strong the motivation to lighten the data volume is. Because the IRD resource is always very small and seldom updated. It's not like cost map and other resources which may achieve hundred MB volume. [[SR]] the calendar attributes are also given in the meta of Server responses. Section 4.1.1., paragraph 5: > This field MUST NOT be specified if member "calendar-attributes" is > not present for this information resource. From section 8.3.7 of RFC7285, "ALTO implementations MUST ignore unknown fields when processing ALTO messages", I think we need to change "This field MUST NOT be specified" to "This field MUST be ignored". Because if "calendar-attributes" is not present, the server should be a legacy ALTO server which doesn't support calendar extension. So the "calendared" field is an unknown field in the request. ==> Indeed, we should minimize the error messages. So how about writing: "This field SHOULD NOT be specified if ++no++ member "calendar-attributes" is present for this information resource. ++ It will be ignored by a legacy ALTO Server, according to section 8.3.7 of RFC 7285. A Calendar-aware Server will return a response with a single cost value as specified in RFC 7285 ++ ". (Jensen) Looks fine for me. Section 4.2.1., paragraph 1: > The extensions to the requests for calendared Endpoint Cost Maps are > the same as for the Filtered Cost Map Service, specified in section > XXXX of this draft. Just a reminder. Don't forget to change "section XXXX". ==> good catch, thanks a lot. === Once we have an update, I would like to proofread the revision. Chears, Jensen
_______________________________________________ alto mailing list [email protected] https://www.ietf.org/mailman/listinfo/alto
