Re: [netconf] Adam Roach's Discuss on draft-ietf-netconf-restconf-notif-13: (with DISCUSS and COMMENT)

[Adam Roach <[e-mail]>]

Thanks for your quick response. I have some answers inline below (and 
have removed the part of the conversation that requires no response).


On 5/16/19 9:27 PM, Reshad Rahman (rrahman) wrote:
> Hi,
>
> Thanks for the review, please see inline.
>
>
> On 2019-05-15, 3:20 AM, "netconf on behalf of Adam Roach via Datatracker" <[e-mail] on behalf of [e-mail]> wrote:
>
>      Adam Roach has entered the following ballot position for
>      draft-ietf-netconf-restconf-notif-13: Discuss
>      
>      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-netconf-restconf-notif/
>      
>      
>      
>      ----------------------------------------------------------------------
>      DISCUSS:
>      ----------------------------------------------------------------------
>      
>      Thanks for all the work that the authors and other contributors have
>      put into this document. I have two comments that need to be addressed
>      before publication, but they should both be very easy to fix.
>      
>      ---------------------------------------------------------------------------
>      
>      
>      §3.3:
>      
>      >  If a publisher fails to serve the RPC request for one of the reasons
>      >  indicated in [I-D.draft-ietf-netconf-subscribed-notifications]
>      >  Section 2.4.6 or [I-D.ietf-netconf-yang-push] Appendix A, this will
>      >  be indicated by "406" status code transported in the HTTP response.
>      
>      This really isn't what 406 means. 406 means "you sent one or more of the
>      'Accept', 'Accept-Charset', 'Accept-Encoding', or 'Accept-Language' header
>      fields, and I can't generate a response that satisfies what you've asked for."
>      
>      For some of the errors listed in the two cited sections, there is a reasonable
>      semantic mapping onto existing HTTP response codes; e.g. the
>      "no-such-subscription" errors could all reasonably map on to HTTP 404.  I'll
>      note that RFC 8040 section 7 performs exactly this kind of mapping, so the
>      approach seems to be consistent with the way that RESTCONF has elected to use
>      HTTP response codes. In fact, this document already maps from the cited errors
>      to error tags already, and that table maps from error-tag to
>      HTTP response codes, so fixing this should be the relatively straightforward
>      exercise of updaing the tables in this section to also include the HTTP response
>      code that RFC 8040 maps to the indicated error-tag. For example:
>      
>           error identity         uses error-tag             HTTP Response
>           ---------------------- --------------             -------------
>           dscp-unavailable       invalid-value              400
>           encoding-unsupported   invalid-value              400
>           filter-unsupported     invalid-value              400
>           insufficient-resources resource-denied            409
>           no-such-subscription   invalid-value              404
>           replay-unsupported     operation-not-supported    501
>      
>           error identity              uses error-tag          HTTP Response
>           ----------------------      --------------          -------------
>           cant-exclude                operation-not-supported 501
>           datastore-not-subscribable  invalid-value           400
>           no-such-subscription-resync invalid-value           404
>           on-change-unsupported       operation-not-supported 501
>           on-change-sync-unsupported  operation-not-supported 501
>           period-unsupported          invalid-value           400
>           update-too-big              too-big                 400
>           sync-too-big                too-big                 400
>           unchanging-selection        operation-failed        500
>      
>      However you choose to address this, if the error isn't related to any of the
>      four header fields I mention above, then you can't specify the use of a 406.
> <RR> Thank you for suggesting status codes for the different errors, much appreciated. I agree that 406 is not appropriate. Regarding 400, my understanding is that it is for invalid syntax, so for errors such as dscp-unavailable or encoding-unsupported, would 409 be more appropriate than 400?


400 is a generic "there is something wrong with your request for which 
there is no more specific error code," and it's used pretty extensively 
by the base RESTCONF specification. 409 generally means that the server 
is in a (typically temporary) state that is incompatible with the action 
indicated by the request.

Once of the things that I took pains to ensure in my suggestions is that 
the HTTP response I suggest above is one of the ones that RFC 8040 
indicates for the corresponding error-tag (even for those cases where I 
don't think RFC 8040 made a strictly accurate choice). I suspect that it 
is best if the RESTCONF notification behavior is kept in sync with the 
base RESTCONF behavior, but I'm not an expert on the whole YANG 
ecosystem, so I may be mistaken.


>      
>      ---------------------------------------------------------------------------
>      
>      §3.4:
>      
>      This section is unclear about how Server-Sent Events are to be used (in
>      particular, they don't say anything about event type to be used). Based on the
>      one example in Appendix A that shows SSE syntax, I'm assuming you probably do
>      not intend to use SSE "event type" fields to distinguish between events in any
>      way.  This would mean that you need to specify that all SSE messages are sent
>      with an event type of "message," which the server may omit (as it is the
>      default specified in the Server-Side Events specification).  This means that
>      clients will need to accept both:
>      
>      data: {
>      data:   "ietf-restconf:notification" : {
>      data:     "eventTime": "2007-09-01T10:00:00Z",
>      data:     "ietf-subscribed-notifications:subscription-modified": {
>      data:       "id": "39",
>      data:       "uri": "https://example.com/restconf/subscriptions/22"
>      data:       "stream-xpath-filter": "/example-module:foo",
>      data:       "stream": {
>      data:          "ietf-netconf-subscribed-notifications" : "NETCONF"
>      data:       }
>      data:     }
>      data:   }
>      data: }
>      
>      ...and...
>      
>      event: message
>      data: {
>      data:   "ietf-restconf:notification" : {
>      data:     "eventTime": "2007-09-01T10:00:00Z",
>      data:     "ietf-subscribed-notifications:subscription-modified": {
>      data:       "id": "39",
>      data:       "uri": "https://example.com/restconf/subscriptions/22"
>      data:       "stream-xpath-filter": "/example-module:foo",
>      data:       "stream": {
>      data:          "ietf-netconf-subscribed-notifications" : "NETCONF"
>      data:       }
>      data:     }
>      data:   }
>      data: }
>      
>      It may be helpful to incorporate the SSE syntax into all of the notification
>      examples in Appendix A (that is, all of the examples in A.2 and A.3). I would
>      recommend a mix of examples with and without "event: message".
> <RR> Andy Bierman (one of the co-authors) mentioned that RESTCONF does not define any values for "event" and pointed us to this text from RFC8040 P72
>     A RESTCONF
>     server SHOULD NOT send the "event" or "id" fields, as there are no
>     meaningful values that could be used for them that would not be
>     redundant to the contents of the notification itself.


Ah, okay. That seems to cover the situation, although it might bear 
reiterating in this document also.


>      
>
>      ----------------------------------------------------------------------
>      COMMENT:
>      ----------------------------------------------------------------------
>      
>      General comment:
>      
>      It's a bit unclear about what the normative relationship between a Server-Sent
>      Events connection and a subscription is intended to be. For example: if I send
>      an RPC command to create a subscription, and then make two different SSE
>      connections to the resulting URL, will they both receive events associated
>      with that subscription? If so, does a TLS heartbeat failure on one of them
>      cause the entire subscription to go away? (If not, what happens?)
> <RR> The resource created via the establish-subscription is for 1 subscription. There can only be 1 GET on the URI at the same time, this does not seem to be explicitly stated anywhere, so probably a good idea to add text to that effect.
>      
>      If I have only one connection related to a subscription, and I close the TCP
>      connection, does that necessarily make the subscription go away? What if I
>      set up a new TCP connection right away after closing the first one? Will
>      that work?
> <RR> The subscription will not go away when the TCP connection is closed. As mentioned at the end of 3.4, the subscription goes away on receipt of delete-subscription/kill-subscription RPCs or on loss of TLS heartbeat. If a client does a GET after closing the first GET, yes this is supposed to work.
>      
>      What if I use RPC to set up a new subscription, and then wait a few minutes
>      before connecting to the subscription URL?
> <RR> That is also supposed to work as long as the subscription wasn't terminated as per section 3.4.
>      
>      I don't think you need to answer all of these corner cases per se (although it
>      would be nice), but minimally a couple of statements that clearly spell out
>      the relationship between subscriptions and the connections used to deliver
>      related events would help implementors figure out the answers to these
>      questions.


Thanks for the explanation; that all makes sense. I suggest you add 
whatever text you feel is appropriate so that other readers understand 
it as well as I do now. :)

Thanks!

/a