Role is represented as string so we cannot tell if the field has been set.

[rhalstea]

I think the intent was for these: https://github.com/p4lang/p4runtime/blob/main/proto/p4/v1/p4runtime.proto#L68 http://go/gh/p4lang/p4runtime/blob/main/proto/p4/v1/p4runtime.proto#L108 https://github.com/p4lang/p4runtime/blob/main/proto/p4/v1/p4runtime.proto#L693

to not be a string, but maybe a: https://github.com/p4lang/p4runtime/blob/main/proto/p4/v1/p4runtime.proto#L693

The problem with using string here is that we cannot tell the difference between role being set as an empty string, and not being set at all (i.e. has_role() is only generated for message fields IIRC), right?

[antoninbas]

We used to have an integer before this change: https://github.com/p4lang/p4runtime/pull/346. So the situation was the same. There is no ambiguity here IMO because we use the default value for the data type (0 when the field was an integer, the empty string now) to designate the "default" role. The "default" role can access all P4 objects for runtime programming: https://p4.org/p4runtime/spec/main/P4Runtime-Spec.html#sec-default-role. We do not have a use case for differentiating between empty string and unset role: I doubt anyone would want to define a custom role with restricted access and give it the empty string as a name. So I don't think wrapping this field in its own Protobuf message is necessary.

[rhalstea]

That makes sense. I missed the part in the spec about an empty string being the default role.

Thanks for clarifying