-
Notifications
You must be signed in to change notification settings - Fork 5
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
Perform API discovery only as needed #8
Perform API discovery only as needed #8
Conversation
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.
Should the items in the gvkToGVR cache ever expire? I didn't see any way for them to be refreshed - it seems like it only gets populated if it wasn't already present.
Generating a REST mapper is very expensive on the Kubernetes API. This approach just gets the information needed for the particular watch request and then caches the information. This relates to this PR: open-cluster-management-io/governance-policy-framework-addon#39 Signed-off-by: mprahl <mprahl@users.noreply.github.com>
I didn't see a problem with this unless something funky happened with the CRDs and the resource was renamed but the kind was kept the same, but regardless, I added a cache expiration time of 10 minutes. |
a8e15f1
to
1692bc1
Compare
There seems to be a bug in the current version. Signed-off-by: mprahl <mprahl@users.noreply.github.com>
adcd3fb
to
55c6761
Compare
Kudos, SonarCloud Quality Gate passed! |
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.
Should the items in the gvkToGVR cache ever expire? I didn't see any way for them to be refreshed - it seems like it only gets populated if it wasn't already present.
I didn't see a problem with this unless something funky happened with the CRDs and the resource was renamed but the kind was kept the same, but regardless, I added a cache expiration time of 10 minutes.
That would be pretty strange, but this seems quite robust now! Thanks for adding that. LGTM.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JustinKuli, mprahl 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 |
Generating a REST mapper is very expensive on the Kubernetes API. This approach just gets the information needed for the particular watch request and then caches the information.
This relates to this PR:
open-cluster-management-io/governance-policy-framework-addon#39