openconfig / gribi

A gRPC Interface to a Network Element RIB.
Apache License 2.0
57 stars 14 forks source link

AFTResult status code for a Modify session w/o param negotiation #88

Open LimeHat opened 1 year ago

LimeHat commented 1 year ago

In a scenario where a client skips session parameters negotiation, and starts with a ModifyRequest with an AFToperation only, should the successful operation be acknowledged with a deprecated OK status as suggested by these lines in .proto https://github.com/openconfig/gribi/blob/0f369fbcc905def3a2e744afb77b5ea4a336b0a2/v1/proto/service/gribi.proto#L96-L102

Or, as could be implied from the specification.md, we should use RIB_PROGRAMMED instead (since this is the default ack mode) https://github.com/openconfig/gribi/blob/0f369fbcc905def3a2e744afb77b5ea4a336b0a2/doc/specification.md?plain=1#L151-L163 ?

robshakir commented 1 month ago

@LimeHat Apologies, missed this issue -- with the deprecation of OK, then RIB_PROGRAMMED should be used IMHO. Any thoughts?

robshakir commented 1 month ago

Currently this is not tested in the compliance test suite, because we always assume that we specify some persistence mode, and SINGLE_PRIMARY. Tying adding the compliance test in https://github.com/openconfig/gribigo/commit/689fdad9f8f1d165acff51b6cbc6da1c0735352e to adding support for non-leader elected, non-persistent clients to gribigo.

LimeHat commented 2 weeks ago

Thanks Rob. I agree with the behavioral part and thank you for adding the test.

Perhaps the proto comment can also be updated to remove the ambiguity? https://github.com/openconfig/gribi/blob/024356c16d0ec63cceced93383bf841b7af5576f/v1/proto/service/gribi.proto#L107