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

feat!: changing cache provider to caffeine over guava #1065

Merged

Conversation

matheusverissimo
Copy link
Contributor

@matheusverissimo matheusverissimo commented Nov 6, 2024

This PR changes the implementation of caching from Guava to Caffeine.

Related Issues

Resolves thomaspoignant/go-feature-flag#1985 from go-feature-flag.

Notes

This change introduces a breaking change on the API, because the property cacheConfig of GoFeatureFlagProviderOptions now expects a Caffeine configuration object instead of a Guava CacheBuilder object.

Testing

This implementation caused test failure on should_resolve_from_cache_max_size in GoFeatureFlagProviderTest.

Unlike Guava, Caffeine does not guarantee that the cache size never exceeds the specified limit at any time due to important differences in the eviction algorithm compared to Guava.

Because of this, the last assertion from this test case was removed.

More about these differences can be found here:
Caffeine Wiki - Guava
ben-manes/caffeine#420 (comment)

Follow-up Tasks

We need to document the breaking changes on the API in:

  1. Go Feature Flag Website

@thomaspoignant I opened a new PR because I messed up with my GPG signature and had to rewrite a lot of commits

@matheusverissimo matheusverissimo requested a review from a team as a code owner November 6, 2024 13:49
@matheusverissimo matheusverissimo changed the title Cache with caffeine feat!: changing cache provider to caffeine over guava Nov 6, 2024
Copy link
Member

@thomaspoignant thomaspoignant left a comment

Choose a reason for hiding this comment

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

@matheusverissimo thanks a lot for this contribution.
I will merge it and it will be part of the next release of the provider.

@thomaspoignant thomaspoignant merged commit 7083586 into open-feature:main Nov 6, 2024
5 of 6 checks passed
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.

(change) Change cache in openfeature java provider
2 participants