search

openconfig / gnmi

gRPC Network Management Interface
Apache License 2.0
459 stars 196 forks source link

commit confirmed #154

Closed hellt closed 9 months ago

hellt commented 11 months ago

This PR adds a proposal for commit confirmed extension.

Extension description is provided in https://github.com/openconfig/reference/pull/198

hellt commented 10 months ago

@dplore does it make sense to ask for the community to cast votes/comment for #154 and #155 to move the needle forward?

dplore commented 10 months ago

@dplore does it make sense to ask for the community to cast votes/comment for #154 and #155 to move the needle forward?

We can take comments now and also add to the agenda for next OC Community review (first Thursday of each month).

The decision to merge will be made by the OC Operators group which has weekly private meetings. We will review and compare both at the Oct 24th occurrence of the OC Operators meeting.

dplore commented 10 months ago

OC Operators reviewed today. There is some slight preference for #154 based on slightly better/more explicit documentation for each field in the proto. The protos seem equivalent in functionality.

An open question for the community is, is #154 is missing anything? Please provide comments in the next two weeks, we'll move to merge this on Nov 7, 2023

kidiyoor commented 10 months ago

It looks like there are two key differences b/w #154 and #155, we should probably clarify for posterity why we are choosing one over the other.

  1. Identifier(ID) generation

As per #154, The Commit ID is generated by the server. Client receives the ID in the response and uses this Commit ID to confirm/cancel the request.

As per #155, that server should not be involved in ID management. Clients should generate and store the ID locally before making the SetRequest, this ID is passed during the request and server registers it, the same ID is used by the clients to confirm/cancel the request.

Strong opinion: There are already systems which deal with configuration generation and have an identifier associated with the generated configuration. We should allow such system to use the same identifier as the commit id. This will enhance the debug ability and visibility of the configuration flow throughout the pipeline.

  1. Use of ENUM vs oneof

I am ok with either, as long as state the reason for the choice.

Opinion: It looks to me that oneof is able to model the three action in a more readable fashion without requiring much explanation. Specially if you look at field like rollback_duration which is only applicable during request, we are able to ensure it in the spec itself. However, this is not a strong opinion, if the community thinks its ok to use ENUM here that's fine.

kidiyoor commented 10 months ago

In support of this claim -There are already systems which deal with configuration generation and have an identifier associated with the generated configuration, I want to point out that there is already precedence for this in NETCONF - https://datatracker.ietf.org/doc/html/rfc6241#section-8.4.1

 If the
   <persist> element is given in the confirmed <commit> operation, a
   follow-up commit and the confirming commit can be given on any
   session, and they MUST include a <persist-id> element with a value
   equal to the given value of the <persist> element.

The server expects client to pass the optional during commit operation.

hellt commented 9 months ago

replaced by #155