-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
KEP-2340: update the semantics table for the list requests #4748
base: master
Are you sure you want to change the base?
KEP-2340: update the semantics table for the list requests #4748
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: p0lyn0mial The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| _RV_ | _NotOlderThan_ | _token_ | _unset_/ _N_ | Fails [validation] | Fails [validation] | | | ||
| ResourceVersion | ResourceVersionMatch | Continuation | Limit | etcd implementation | watchcache implementation | changed | | ||
|-----------------|----------------------|-------------------|---------------|-----------------------------------------|----------------------------------------------------------------------------------|----------| | ||
| _unset_ | _unset_ | _unset_ | _unset_ | Quorum read request | Read etcd RV. Wait for cache synced to _RV_+ and list from cache | Yes | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i've changed this row.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/kubernetes/enhancements/pull/4748/files?w=1 helps reviewing :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is misleading. We don't want to change the lines to how it works now - the whole idea was to have a table that shows "how it worked before" and mark columns that will be changed.
So we don't want to change the column you changed, if anything we want to change the last column to explain how it works now.
[Same below]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I will add description to the last column then. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pushed, ptal.
| ResourceVersion | ResourceVersionMatch | Continuation | Limit | etcd implementation | watchcache implementation | changed | | ||
|-----------------|----------------------|-------------------|---------------|-----------------------------------------|----------------------------------------------------------------------------------|----------| | ||
| _unset_ | _unset_ | _unset_ | _unset_ | Quorum read request | Read etcd RV. Wait for cache synced to _RV_+ and list from cache | Yes | | ||
| _unset_ | _unset_ | _unset_ | _N_ | Quorum read request | Read etcd RV. Wait for cache synced to _RV_+ and list up to "N" items from cache | Yes | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i've changed this row.
| _RV_ | _Exact_ | _unset_ | _unset_ / _N_ | Read request from RV=_RV_ | Delegated to etcd | | | ||
| _RV_ | _Exact_ | _token_ | _unset_ / _N_ | Fails [validation] | Fails [validation] | | | ||
| _RV_ | _NotOlderThan_ | _unset_ | _unset_ | Quorum read request + check for _RV_ | Wait for cache synced to _RV_+ and list from cache | | | ||
| _RV_ | _NotOlderThan_ | _unset_ | _N_ | Quorum read request + check for _RV_ | Wait for cache synced to _RV_+ and list up to "N" items from cache | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i've changed this row.
/assign @wojtek-t |
d05326c
to
1c17ab2
Compare
No description provided.