openconfig / reference

This repository contains reference implementations, specifications and tooling related to OpenConfig-based network management.
Apache License 2.0
155 stars 88 forks source link

Disallow using replace(nil) as a deletion mechanism. #171

Closed wenovus closed 2 years ago

wenovus commented 2 years ago

This corresponds to the last point in https://github.com/openconfig/reference/issues/155#issuecomment-1154173549

dplore commented 2 years ago

Is it obvious that a client is not permitted to provide invalid values? Or have we seen this as an issue in the past?

wenovus commented 2 years ago

It is not obvious. There have been instances where Replace(nil) are being used in existing code: https://github.com/openconfig/featureprofiles/blob/4b57f298252aac7ec24f6bd99c668959308844b6/feature/bgp/gracefulrestart/ate_tests/bgp_graceful_restart_test/bgp_graceful_restart_test.go#L381 This is to prohibit this.

Alternatively we can say the server SHOULD reject it instead of a MUST. I think this also makes sense because the purpose of this as stated is to prevent a programming error, which is a style issue rather than a functional issue.

dplore commented 2 years ago

Ok, thanks for the reference. I think this is a good clarification.