api: add attention to HeaderValueOption.append to avoid removing this field#23518
Conversation
… field Signed-off-by: wbpcode <wangbaiping@corp.netease.com>
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
| // is always APPEND_IF_EXISTS_OR_ADD. | ||
| // It means if we remove this field, the default behavior in these filter will be changed and may break our | ||
| // users. | ||
| // So we should keep this field and couldn't remove it until the above issue is resolved one day. |
There was a problem hiding this comment.
Emm...The last two sentences seems like a note for the developer, it shouldn't be part of the API document.
There was a problem hiding this comment.
It's attention to tell developers never remove this field in the future even its marked as deprecated.
I couldn't find a better position for this attention.
There was a problem hiding this comment.
As I mentioned in Slack, at this point the APIs are forever. We are never removing anything, so I think we can remove this. Can we rewrite this as end user documentation? Thank you.
/wait
There was a problem hiding this comment.
It's easy to do some comments update. But I think there still is some info gap. 🤣 I re-read the API version policy. There are some conclusions by my understanding. Feel free to point it out if I am wrong.
- When we say
the APIs are forever, it means that this field is forever in thecore.v3.HeaderValueOption. - One day the v4 APIs is enabled, then this field will be removed from the
core.v4.HeaderValueOption. But the break changes are acceptable for major API versions update.
There was a problem hiding this comment.
I haven't gotten around to doing an official policy update, but at this point realistically a) we are never doing v4 and b) we are never actually deleting any fields/code from Envoy. It's too disruptive. cc @envoyproxy/api-shepherds
phlax
left a comment
phlax
left a comment
There was a problem hiding this comment.
couple of format nits inline...
Co-authored-by: phlax <phlax@users.noreply.github.com> Signed-off-by: code <wangbaiping@corp.netease.com>
Co-authored-by: phlax <phlax@users.noreply.github.com> Signed-off-by: code <wangbaiping@corp.netease.com>
Signed-off-by: wbpcode <wangbaiping@corp.netease.com>
Signed-off-by: wbpcode <wangbaiping@corp.netease.com>
| // It means that the default behavior of ``HeaderValueOption`` in these two services will be changed after | ||
| // this field is removed in the future version of API. |
There was a problem hiding this comment.
Per other comments, it's not going to get removed, so let's remove this. It's still not clear to me what we are telling the end user. Are we just telling them that the default is false when used in these 2 services? If so can we just say that clearly?
Co-authored-by: Matt Klein <mattklein123@gmail.com> Signed-off-by: code <wangbaiping@corp.netease.com>
Signed-off-by: wbpcode <wangbaiping@corp.netease.com>
5 participants
Signed-off-by: wbpcode wangbaiping@corp.netease.com
Commit Message: api: add attention to HeaderValueOption.append to avoid removing this field
Additional Description:
See #22845.
Risk Level: n/a.
Testing: n/a.
Docs Changes: added.
Release Notes: n/a.
Platform Specific Features: n/a.