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

vendor: bump golang-lru to v2 (requires Go >= 1.18 support for generics) #22644

Merged
merged 1 commit into from Dec 9, 2022

Conversation

rolinh
Copy link
Member

@rolinh rolinh commented Dec 9, 2022

The golang-lru package is used in Hubble's L7 parser to maintain a cache of trace ID and timestamp.

The new major version of github.com/hashicorp/golang-lru uses Go generics and thus Go >= v1.18 is required (this is already enforced via the go.mod file). Switching to generics means breaking changes in the package's API as, when declaring a new lru.Cache or invoking lru.New, the type of keys and values must be specified. However, this means that we can know trust the types of the key/value pairs we have in the cache and don't need to do any type assertion.

The golang-lru package is used in Hubble's L7 parser to maintain a cache
of trace ID and timestamp.

The new major version of github.com/hashicorp/golang-lru uses Go
generics and thus Go >= v1.18 is required (this is already enforced via
the go.mod file). Switching to generics means breaking changes in the
package's API as, when declaring a new lru.Cache or invoking lru.New,
the type of keys and values must be specified. However, this means that
we can know trust the types of the key/value pairs we have in the cache
and don't need to do any type assertion.

Signed-off-by: Robin Hahling <robin.hahling@gw-computing.net>
@rolinh rolinh added release-note/misc This PR makes changes that have no direct user impact. sig/hubble Impacts hubble server or relay labels Dec 9, 2022
@rolinh rolinh requested review from a team as code owners December 9, 2022 09:22
@rolinh rolinh requested a review from tklauser December 9, 2022 09:22
Copy link
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

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

Nice!

@rolinh
Copy link
Member Author

rolinh commented Dec 9, 2022

Travis tests pass and the changes are only covered by Hubble unit tests. Marking as ready to merge.

@rolinh rolinh added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Dec 9, 2022
@pchaigno pchaigno merged commit ba1fd7d into master Dec 9, 2022
@pchaigno pchaigno deleted the pr/rolinh/golang-lru-v2 branch December 9, 2022 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact. sig/hubble Impacts hubble server or relay
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants