Subject: [Ecrit] draft-ietf-ecrit-lost-planned-changes-05

From: "Randall Gellens" <[email-masked]>
Date: Thu, 09 Dec 2021 19:41:02 -0800

Below is a list of both substantive and editorial comments:

(1) In the first paragraph of Section 1 (Introduction), consider some 
rewording for clarification:

Old:

    Prior to the change, a Presence Information Data
    Format Location Object (PIDF-LO) specifying a civic location in the
    affected area would have a blank A3 element, or would contain some
    other value; after the change, a PIDF-LO specifying the same 
location
    would contain an A3 element set to the name of the municipality that
    annexed that part of the county/district.

New:

    Prior to the change, a valid Presence Information Data Format
    Location Object (PIDF-LO) specifying a civic location in the
    affected area might have an A3 element that is blank or contains
    the name of some other municipality; after the change, a valid
    PIDF-LO specifying the same location will contain an A3 element
    set to the name of the municipality that annexed that part of the
    county/district.

(2) In the second paragraph of Section 1, please expand "LIS" prior to 
its first use, and also consider adding s small bit of explanatory text.

Old:

    Records in a LIS

New:

    A Location Information Server (LIS) maintains location
    information on devices/clients and typically uses the Location to
    Service Translation [LoST] protocol [RFC5222] to validate its civic
    location records.  Records in a LIS

(3) Typo in the third paragraph of Section 1, an omitted apostrophe:

Old:

   which of the clients records may change as a result of the planned

New:

   which of the client's records may change as a result of the planned

(4) Please clarify what "IDs are ordered." means in the third paragraph 
of Section 1. E.g., are they integer values that increases with each 
subsequent ChangeSet, or an opaque string or value that is meaningful 
only to the server that issued it, but that is different for each 
subsequent ChangeSet, or something else? May also want to add text to 
clarify if two LoST servers that have the same data also have the same 
change IDs, or if each server can have its own ID even for the exact 
same change list.

(5) Also in the third paragraph of Section 1, please consider changing 
both instances of "the" to "a" in the text "allows the client to poll 
the server".

(6) In the fifth paragraph of Section 1, there's an outdated reference 
to the notification URI. I suggest deleting it or making the text a 
comment in the XML, pending final resolution of the issue.

Old:

    The notification
    URI allows a LIS to be alerted to planned changes, while the "as of"

New:

    The "as of"

(7) Continuing from (6), in the 7th paragraph of Section 1:

Old:

    In situations where it is not practical or advisable for the LIS to
    maintain a stable URI for each of its records, periodic revalidation
    can still be used to maintain the data in the LIS.

New:

    In addition to the poll interface, periodic revalidation can
    still be used to maintain the data in the LIS.

(8) While there's a typo in the 7th paragraph of Section 1, a missing 
comma after "<findServiceResponse>" in the text "the 
<findServiceResponse> which provides", it might be more clear to replace 
"which" with "to" (and drop the "s" from "provides"), as you prefer:

Old:

    to the <findServiceResponse> which provides advice from the server 
to

New:

    to the <findServiceResponse>, which provides advice from the server 
to

Or:

    to the <findServiceResponse> to provide advice from the server to

(9) In the first paragraph of Section 3, consider editorial tweaks to 
the description of GetChangeSet:

Old:

    the ChangeSet object which contains the

New:

    the corresponding ChangeSet object. A ChangeSet object contains an

(10) Continuing (9), to make it more clear that what's returned is an 
array of arrays:

Old:

    A partial location is

New:

    Each partial location is

(11) Typo in the middle of the first paragraph of Section 3: "clients" 
needs an apostrophe.

(12) Another typo in the first paragraph of Section 3, delete "a" before 
the address element list:

Old:

    have a Country

New:

    have Country

(13) Typo in the second paragraph of Section 3: "is string" should be 
"is a string".

(14) Suggested rewording in the second paragraph of Section 3:

Old:

    A new client does not know any
    ids, or a client may lose the id that it had.  The client would poll
    omitting the changeSetId in the poll query, and it response, the
    server returns all the ChangeSets it knows about

New:

    Since a new client does not have a previous changeSetId, and to
    accommodate a client that lost its previous changeSetId, the
    changeSetId is optional in the PlannedChangePoll interface.  When
    used without a changeSetId, the server returns a list containing
    each changeSetId for which it has information.

(15) Typo in the second paragraph of Section 3: "whoose" should be 
"whose".

(16) Another in the second paragraph of Section 3: "recieve" should be 
"receive".

(17) Consider adding text to or following the second paragraph of 
Section 3 to address the situation where a "tardy" client has missed 
change sets that the server has aged out. One option would be to require 
the server to keep a tombstone containing the newest change set ID that 
it has deleted. If a client uses PlannedChangePoll with a changeSetId 
that's equal to or older than the tombstone ID, the server returns a 
warning (e.g., "agedOutChangeSetId") to alert the client that it has 
missed change sets and should revalidate its entire database. The server 
should probably include this warning if the client omits the changeSetId 
and the server has aged out one or more change sets. To my reading, the 
way a client recovers or bootstraps is that it uses the 
PlannedChangePoll to obtain the latest changeSetId the server has and 
then revalidates its entire database. From then on, using 
PlannedChangePoll with the latest changeSetId it has will keep the 
client updated unless it gets so tardy that it receives the 
"agedOutChangeSetId" warning. (It needs to obtain the latest changeSetId 
before it revalidates its database since change sets could be added 
during the revalidation.)

(18) In Section 3, there's text:

    Versions are
    described as a major version, the period "." and a minor version,
    where major and minor versions are integers.

When I read this, I thought this meant that the version interface 
returns the two integers as a single string (e.g., "3.2"), but in the 
schema I see the interface returns them as integers. If I understand the 
intent of the text here (and I could easily be wrong), consider 
rewording as:

    Versions are described in this text as a major version, a period
    ("."), and a minor version, where the major and minor versions
    are integers. The versions interface returns the major and minor
    components of each version as two discrete integers in a versions
    object.

The interface should probably also allow a server ID string to be 
returned, strictly for logging/debugging. The text in Section 3 should 
then mention this.

(19) Section 3: the description of the version mechanism doesn't explain 
how a client uses a specific version. I see in Section 8 that the 
version is part of the path; the text should say this and also have some 
mechanism to guarantee interoperability, such as requiring clients and 
servers that implement the extension to always support version 1.

(20) In Section 4 (<plannedChange> element), are there any restrictions 
on the 'asOf' time and date? Can it be in the past (e.g., to check if a 
location was valid on the date of some past provisioning) or years into 
the future? Should there be a warning or error if the 'asOf' date is out 
of whatever range is acceptable?

(21) Also in Section 4, should there be a warning if the LoST server 
doesn't have information specific to the 'asOf' time and date, and is 
instead validating as if 'asOf' wasn't specified? (E.g., the 'asOf' date 
is before the next planned change, or is far past the last planned 
change.)

(22) Also in Section 4, the text should say that 'asOf' is mandatory 
(the schema in Section 7 makes it mandatory, but this should be stated 
in the text).

(23) Typo in Section 4: capitalize "Element" in the section name.

(24) Typo in Section 5, first paragraph: delete semicolon after 'ttl' 
("for the 'ttl';").

(25) Section 5, second paragraph, I suspect the second use of "server" 
is an error and should be "client" in the text:

    Too short, and load on the server may overwhelm
    it.  Too long and invalid data may persist in the server for

(26) Also in Section 5, second paragraph, last sentence: same issue as 
(6) et al, reference to notification mechanism that should be deleted or 
commented-out in the XML pending acceptance of this mechanism.

(27) Section 5, third paragraph, same issue as (6) et al, I suggest 
deleting or commenting out the comma through the end of the sentence in 
the text:

    ,
    especially if the URI mechanism is widely deployed at both the 
server
    and the clients.

(28) Section 5, third paragraph, in the last line, I suggest changing 
"would be" to "might be".

(29) Section 6, first paragraph, typo "exected" should be "expected".

(30) Section 7, typo in first line of first paragraph: suggest changing 
"prior section schema" to either "prior section's schema" (with a 
possessive) or "schema in the prior section" (personally, I think the 
latter reads better, but either is fine).

(31) Section 7, at the end, same issue as (6) et al, I suggest deleting 
or commenting out in the XML the text:

  <!-- extend the extensionPoint of extensionContainer to include: -->
             <xs:element ref="uriNotStored"/>
  <!-- where -->
             <xs:element name="uriNotStored" type="basicException"/>

(32) Section 8, '/PlannedChangePoll:', the summary "Get a list of 
planned changeSetIds" could be read as "Get a list of changeSetIds for 
future changes" when I think it is "Get a list of existent changeSetIds, 
possibly with older ones omitted due to having been aged out". I think 
this text should be clarified.

(33) Same as above, the '200' OK description of "description: Planned 
Changes" should be clarified, perhaps something such as "description: 
existing changeSetIds".

(34) Same as above, should "operationId: getPlannedChangeIds" be 
"operationId: getChangeIds" or some such?

(35) Similar to above, should "PlannedChangeIdList" be "ChangeIdList"?

(36) More fundamentally, are change sets restricted to changes that have 
not yet occurred? The names as noted above suggest so, as once the 
changes occur they aren't "planned," whereas I think of them as set of 
changes that either have already occurred or are still planned, so a 
"tardy client" can catch up.

(37) Similarly, the description for "changeSetEffective" reads "date and 
time change will come into effect", implying only future changes. I'd 
suggest "effective date and time of the change".

(38) In Section 9, same as (6) et al, there is an outdated mention of 
the notifications URI; I suggest deleting (or commenting-out) everything 
in the first paragraph after the first sentence, and the entire second 
and third paragraphs. You might then want to add some text about the 
polling interface, if you think anything about it merits mention.

(39) In Sections 10.1 and 10.2, just to make things easier and more 
error-resistant, you might want to add a comment at the end of each 
section that says 'IANA: Please replace "????" above with the RFC number 
of THIS DOCUMENT', because "THIS DOCUMENT" is commonly used as the key 
so might be searched for.

--Randall