p4lang / p4runtime

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

Clarify that write interts can use more specific error codes. #347

Closed rhalstea closed 3 years ago

rhalstea commented 3 years ago

Since MODIFY allows for more specific error codes where applicable it seems like the intent may have been to allow INSERTS the same discretion?

stefanheule commented 3 years ago

Some examples:

rhalstea commented 3 years ago

Thanks Antonin & Stefan,

The NOT_FOUND for set_nexthop_id example also works on insert, and could signal the controller isn't ordering it's requests properly.

antoninbas commented 3 years ago

Some examples:

  • Insert of an action that doesn't exist: NOT_FOUND.

I would be more tempted to return INVALID_ARGUMENT here, and save NOT_FOUND for entities created at runtime by the controller

  • Modify of a table entry that uses an action set_nexthop_id where the nexthop doesn't exist (some targets may have checks like this): NOT_FOUND.

That one makes sense to me, although it is target-specific

stefanheule commented 3 years ago

Some examples:

  • Insert of an action that doesn't exist: NOT_FOUND.

I would be more tempted to return INVALID_ARGUMENT here, and save NOT_FOUND for entities created at runtime by the controller

Why is that? Seems like exactly what NOT_FOUND is supposed to be used for, no? Generally INVALID_ARGUMENT should only be used if there is no more specific code.

antoninbas commented 3 years ago

@stefanheule You're right. If the action doesn't exist at all, I would return NOT_FOUND. If the action exists but is not in the action list for that table, I would return INVALID_ARGUMENT.