Update README with draft of spec #1
Conversation
proto/gndp.proto
Outdated
| } | ||
|
|
||
| message TLV { | ||
| string oui = 1; |
There was a problem hiding this comment.
Regarding the construction of the TLV itself, can you elaborate w/ what you have here vs. what may be needed?
The TLV breakdown here I think would be:
- Determine which unused "type" to encode within the 7-bits we have to work with (could vary per platform if custom types are already used) so id imagine we at least need to have our smallest proto3 type
uint32here w/ comment to fall within the reserved range at a minimum - Length can be calculated by the system - doesn't necessarily need to be included in the API
- Subtype and the Value: If we want to be opaque here, maybe we say the subtype is 8-bits (uint32), maybe defined out-of-band? - value can stay as is as string
Thoughts?
There was a problem hiding this comment.
changed this values to be uint32
type is assumed to be 127 for all custom tlv's (i don't expect to set any reserved tlv's as part of this spec
| repeated TLV tlvs = 1; | ||
| } | ||
|
|
||
| message SetResponse{ |
There was a problem hiding this comment.
Any operation that does not succeed we probably want to figure out proper error code mapping along w/ messaging
There was a problem hiding this comment.
what does that have to do with an empty reply message
any errors are returned as a grpc error from the rpc itself
There was a problem hiding this comment.
Just the best spot to tag a comment is all...
If empty then it implies gRPC errors and will probably need to add a mini-specification or within the IDL comments a table much like what we have in the gNMI spec
There was a problem hiding this comment.
that isn't how errors should be handled in grpc
There was a problem hiding this comment.
https://github.com/grpc/grpc-go/blob/master/examples/features/error_handling/client/main.go
would be a simple example
There was a problem hiding this comment.
I'm referring to tightening a mapping of behaviors -> gRPC error codes like
https://github.com/openconfig/reference/blob/master/rpc/gnmi/gnmi-specification.md#334-getresponse-behavior-table
https://github.com/openconfig/reference/blob/master/rpc/gnmi/gnmi-specification.md#3524-subscriberesponse-behavior-table
At the end of the day, this is an API specification so if not defined will be left up to interpretation which may or may not be obvious and likely divergence in implementation behavior
There was a problem hiding this comment.
grpc codes are standard on uses.
https://grpc.github.io/grpc/core/md_doc_statuscodes.html
gndp itself is not using those codes in any way that would be different from those values
i.e.
the server should return unimplemented if it isn't implemented
Internal if the server's implementation has an internal error and so on?
i am not sure what needs to be elaborated here?
| srcs = ["gndp.proto"], | ||
| ) | ||
|
|
||
| go_proto_library( |
There was a problem hiding this comment.
Need to add compilers = ["@rules_go//proto:go_grpc"],. gndp.pb.go is missing GRPC services. AFAICT you only need to use the GRPC compiler (using go_proto and go_grpc generates an error).
| @@ -0,0 +1,15 @@ | |||
| # Copyright 2024 Google LLC | |||
There was a problem hiding this comment.
Do we want to add a .bazelversion file to pin 7.2.0?
No description provided.