rpc: improve docs for blockingQuery #12343
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dnephin
added
the
pr/no-changelog
dnephin
force-pushed
the
dnephin/blocking-query-docs
branch
from
427513d
to
a4f8028
Compare
dnephin
force-pushed
the
dnephin/blocking-query-docs
branch
from
a4f8028
to
a5de2b8
Compare
freddygv
approved these changes
Feb 15, 2022
agent/consul/rpc.go
Outdated
| // The query function must follow these rules: | ||
| // | ||
| // 1. to access data it must use the passed in state.Store. | ||
| // 2. it must set the responseMeta.GetIndex to an index greater than |
There was a problem hiding this comment.
It could be clearer to reference the field name that gets updated:
Suggested change
| // 2. it must set the responseMeta.GetIndex to an index greater than | |
| // 2. it must set the responseMeta.Index to an index greater than |
There was a problem hiding this comment.
Ya, I think this reads better. It's slightly less precise since technically we don't know what the field name is in the interface, but hopefully the reader understands the implication.
agent/consul/rpc.go
Outdated
| // To ensure optimal performance of the query, the query function should make a | ||
| // best-effort attempt to follow these guidelines: | ||
| // | ||
| // 1. only set responseMeta.GetIndex to an index greater than |
Follow the Go convention of accepting a small interface that documents the methods used by the function. Clarify the rules for implementing a query function passed to blockingQuery.
dnephin
force-pushed
the
dnephin/blocking-query-docs
branch
from
a5de2b8
to
3301f94
Compare
|
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/585305. |
This was referenced Feb 15, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Clarify the rules for implementing a query function passed to
Server.blockingQuery. Related to #12109 and #12110The new interfaces here are to follow the Go convention of accepting a small interface that documents the methods used by the function. They help document the function by being explicit about which methods are used.
In the future we can probably removes
structs.QueryOptionsCompatandstructs.QueryMetaCompatsince those are now only used by a few parse functions inhttp.go. By following the pattern used here we could replace that usage as well.