-
Notifications
You must be signed in to change notification settings - Fork 386
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
✨ pkg/indexers: add helers for cache server fallback #2944
✨ pkg/indexers: add helers for cache server fallback #2944
Conversation
Every single consumer of these functions operating over replicated data needs to use the same exact pattern to access their data. There's no need to make them all duplicate the logic. Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ncdc The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Test failed on ... DNS? |
// ByPathAndNameWithFallback returns the instance of T from the indexer with the matching path and name. Path may be a canonical path | ||
// or a cluster name. Note: this depends on the presence of the optional "kcp.io/path" annotation. If no instance is found, globalIndexer | ||
// is searched as well. Any errors short-circuit this logic. | ||
func ByPathAndNameWithFallback[T runtime.Object](groupResource schema.GroupResource, indexer, globalIndexer cache.Indexer, path logicalcluster.Path, name string) (ret T, err error) { |
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.
Why do we need both ByIndexWithFallback
and ByPathAndNameWithFallback
methods?
It seems that both methods eventually make use of indexer.ByIndex
:)
The difference seems to be that ByIndex
returns an array of object whereas ByPathAndName
returns a single object.
Do we need this distinction or could we unify into one method?
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.
@p0lyn0mial please feel free to propose a simpler factoring! One was not obvious to me since the error checking is different.
Every single consumer of these functions operating over replicated data needs to use the same exact pattern to access their data. There's no need to make them all duplicate the logic.
/cc @nrb @sttts
/assign @p0lyn0mial