Skip to content
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

Add GetCachedSupportedFeatures method to hcn package #1123

Merged
merged 4 commits into from
Aug 20, 2021

Conversation

dcantah
Copy link
Contributor

@dcantah dcantah commented Aug 19, 2021

To avoid a breaking change on GetSupportedFeatures introduce a new
GetCachedSupportedFeatures method. This method does the feature check
and version parsing once and then assigns a global with the information.

This can be used to optimize for situations where many uses of the
hcn.IsXSupported methods are going to be used (kube-proxy for example).
Previously they would call GetSupportedFeatures and end up re-querying and
revalidating the feature ranges every time.

Signed-off-by: Daniel Canter dcanter@microsoft.com

To avoid a breaking change on GetSupportedFeatures introduce a new
GetCachedSupportedFeatures method. This method does the feature check
and version parsing once and then assigns a global with the information.
This can be used to optimize for situations where many uses of the
hcn.IsXSupported methods are going to be used (kube-proxy for example).

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
@dcantah dcantah requested a review from a team as a code owner August 19, 2021 03:15
@dcantah
Copy link
Contributor Author

dcantah commented Aug 19, 2021

@kevpar @msscotb Feedback addressed, ptal when you get another chance. Thanks!

- Change versionErr -> featuresErr
- Move work inside of the sync.Once out to its own function so we
can use standard return err error handling and simply assign the
output in GetCachedSupportedFeatures
- Mark GetSupportedFeatures as deprecated.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
@dcantah dcantah force-pushed the new-hcn-features-call branch from 5cd55e5 to f78d0ff Compare August 19, 2021 18:01
Signed-off-by: Daniel Canter <dcanter@microsoft.com>
return supportedFeatures, featuresErr
}

// Deprecated: Use GetCachedSupportedFeatures instead.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the main comment needs to be first or it will be flagged as not following function comment guidelines.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
Copy link
Member

@kevpar kevpar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants