Re: [regext] Artart last call review of draft-ietf-regext-epp-registry-maintenance-17

[Jody Kolker <[e-mail]>]

Thanks Harald for the detailed review of the document.  Please see our comments in line below.

Thanks again!
Jody Kolker

-----Original Message-----
From: Harald Alvestrand via Datatracker <[e-mail]> 
Sent: Monday, September 13, 2021 3:14 AM
To: [e-mail]
Cc: [e-mail]; [e-mail]; [e-mail]
Subject: Artart last call review of draft-ietf-regext-epp-registry-maintenance-17

Caution: This email is from an external sender. Please do not click links or open attachments unless you recognize the sender and know the content is safe. Forward suspicious emails to isitbad@.



Reviewer: Harald Alvestrand
Review result: Almost Ready

I have reviewed this document as part of the ARTART review team.

Verdict: Almost ready
There are a few things that need better definitions to be comprehensible enough for interoperable implementation. There is also one confusing formatting error that should be fixed before publication.

Weak definitions:

- "Maintenance event" is never defined. From context, it is possible to infer that a maintenance event refers to some service being partially or wholly unavailable in the time interval given; given that this is the whole point of the document, this should be explicit. 

<< 
We will add the following to the first paragraph under "1. Introduction"

"Registries routinely update systems to ensure a higher quality of service, implement new services, or upgrade protocols to the newest standards.  These updates are pushed to various registry environments during time frames that are communicated to registrars as "maintenance events".  Registries usually inform registrars for maintenance events in various formats, none of which …..
"
>>

It should also be explicit that the service will be either fully and correctly available or not available at all, and that no harm (apart from being denied service) will come from trying to access the service in the maintenance interval; "maintenance" that, for instance, puts a test database up where the normal database is would be just a broken service, not "maintenance" in this sense. (If broken stuff might happen, I think you need a new impact value in addition to "full", "partial" or "none"
- something like "STAYAWAYYOUWILLREGRETEVENTHINKINGABOUTTRYING").

<<  In the case described above and any other similar maintenance events, the registry should not be allowing registrars to connect to the system by taking a "full" maintenance.  I don't believe we have ever experienced any maintenance event of the type described above.
>>

- "maint:connection" and "maint:implementation" make very little sense as described. They may refer to having to reconnect the EPP service or to upgrade the EPP schema in use, but since the "maint:name" element of "maint:system"
seems to encompass WHOIS and others, the actions that may be required are not clear; an instruction to "do something connection-related" cannot be interoperably implemented. Suggestion: Either delete these elements or (if intended to be consumed by a human) add the option to add a text description of what should be done.

<<  These flags are only meant to indicate that the registrar should review the maint:description element in the response for further details explaining actions the registrar will be effected by regarding the maintenance.  Adding a description to both of these elements seems to duplicate the information that should be in the maint:description element.  We would like to suggest this edit to the document:
From:
<maint:connection>
            The value SHALL be boolean and indicates if a client needs
            to do something that is connection-related, such as a
            reconnect.

         <maint:implementation>
            The value SHALL be boolean and indicates if a client needs
            to do something that is implementation-related, such as a
            code change.

To:
       <maint:connection>
            The value SHALL be boolean and indicates if a client needs
            to perform an action that is connection related, such as a reconnect.  
	The attribute should only be used as a flag to indicate connections 
	will be affected.  Servers SHOULD include a description of how the 
	connetions are affected in the <main:description> element above.

         <maint:implementation>
            The value SHALL be boolean and indicates if a client needs
            to do perform an action thate is implementation related, 
	such as a code change.  	The attribute should only be used as a flag 
	to indicate connections 	will be affected.  Servers SHOULD include 
	a description of how the implmenation is affected in the 
	<main:description> element above.
>>

- pollType seems somewhat strange. The implicit definition seems to be that the client polls the server and the server replies with a list of outstanding maintenance events, with the value "create" returned the first time the server tells the client about the event. This implies that the server is required to keep state of what it has told each client about the event; same goes for event deletion. If such a state tracking requirement is indeed intended, this should be explicit.

<<
This type of tracking requirement was not intended.  Registries will not be expected to keep state of what each registrar has received as far as the type of maintenance events.
>>

Formatting issues:

In the list of elements in section 3, the indentation of <maint:environment> and the succeeding elements indicates that it is an element of <maint:systems>.
Examples indicate that it is an element of <maint:item>, which makes a lot more sense.

<<
Formatting will be updated.
>>

Precision in definition issues:

The incantation "The extended date-time form using upper case "T" and "Z"
characters defined in ISO 8601 [RFC3339] MUST be used to represent date-time values." is not precise (I don't know if it's common) - it seems to claim that RFC 3339 is ISO 8601, which is just confusing. Suggested format: "The date-time format defined as "date-time" in [RFC3339], with time-offset="Z", MUST be used".

<< 
Text will be updated.
>>

Styllistic issues:

The cuteness of using "upDate" as both meaing "update" and "this is a date"
hurts the eyes :-) Unless there is tradition for this name, I'd suggest being boring and using "updateDate".

<<
This document is following the same pattern used to signify the date the state of a domain was updated,  <domain:update> from RFC 5731, which we considered to be the standard to be followed.
>>

Having migration considerations before item descriptions looks a bit weird when reading the document top to bottom. Would it be nicer to move it after section 4?

<<
The document is following the same format that is used in RFC 8807 and RFC 8847 where migrations to the new version of the extension are covered immediately following the introduction.  However, we will update this document to move the migration section to after section 4 if this is what is needed.
>> 

I have not attempted to verify the schema, nor have I attempted to check the document against common styles for EPP extensions. If comments touch on things that are mandated by common EPP practices, feel free to consider these comments overridden.

Hope this is helpful.