p4lang / p4runtime

Specification documents for the P4Runtime control-plane API
Apache License 2.0
144 stars 88 forks source link

spec doesn't mention whether annotation_locations is optional #311

Closed blp closed 3 years ago

blp commented 4 years ago

The spec talks about annotations and annotation_locations and I cannot see how it says clearly one way or another whether annotation_locations is mandatory for each annotation. If locations are meant to be optional, it would be nice to say so. (And if they're meant to be mandatory, then https://github.com/p4lang/p4c/pull/2419 seems more important.)

antoninbas commented 4 years ago

The intention was definitely to have them be optional (after all, one can have a P4Info message, with annotations, without a corresponding P4 program), but agreed that this needs to be called out explicitly. Spec should say that annotation_locations should either be empty, or its length must match the length of the annotations field. Similarly, for structured annotations, we should state explicitly that the source_location field in the StructuredAnnotation message may be left unset.

antoninbas commented 4 years ago

We will review this in September at the next P4 API WG meeting.

antoninbas commented 4 years ago

Decision at the 09/11/2020 WG meeting was to adopt the proposal in https://github.com/p4lang/p4runtime/issues/311#issuecomment-681204126 and update the spec accordingly.