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

refactor: replace experimental maps and slices with stdlib #6372

Merged
merged 5 commits into from
Dec 4, 2024

Conversation

Juneezee
Copy link
Contributor

@Juneezee Juneezee commented Nov 28, 2024

The experimental functions are now part of the standard library as of Go 1.21 and Go 1.23.

The key difference is that maps.Keys and maps.Values in the golang.org/x/exp package return a slice, whereas maps.Keys and maps.Values in the standard library return an iterator. To work with slices, we need to use slices.Collect to convert the iterator into a slice.

Reference: https://go.dev/doc/go1.21#slices
Reference: https://go.dev/doc/go1.23#iterators

Checklist

The experimental functions are now available in the standard library in
Go 1.21 [1] and Go 1.23 [2].

[1]: https://go.dev/doc/go1.21#slices
[2]: https://go.dev/doc/go1.23#iterators

Signed-off-by: Eng Zer Jun <[email protected]>
@Juneezee Juneezee requested a review from a team as a code owner November 28, 2024 10:26
Signed-off-by: Eng Zer Jun <[email protected]>
@wozniakjan
Copy link
Member

wozniakjan commented Dec 2, 2024

/run-e2e internal|opentelemetry_metrics
Update: You can check the progress here

Copy link
Member

@wozniakjan wozniakjan left a comment

Choose a reason for hiding this comment

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

lgtm, thank you!

@wozniakjan
Copy link
Member

wozniakjan commented Dec 4, 2024

/run-e2e internal|opentelemetry_metrics

I just updated to latest main to check if this also works ok with #6378
Update: You can check the progress here

@wozniakjan
Copy link
Member

I thought it might happen that we will need to bump go back to 1.23

https://github.com/kedacore/keda/actions/runs/12155644728/job/33897625757?pr=6372

pkg/scalers/scalersconfig/typed_config.go:206:18: slices.Sorted requires go1.23 or later (file is go1.22)
pkg/scalers/scalersconfig/typed_config.go:206:30: maps.Keys requires go1.23 or later (file is go1.22)
pkg/scalers/scalersconfig/typed_config.go:464:21: slices.Sorted requires go1.23 or later (file is go1.22)
pkg/scalers/scalersconfig/typed_config.go:464:33: maps.Keys requires go1.23 or later (file is go1.22)
make: *** [Makefile:147: vet] Error 1

@wozniakjan wozniakjan force-pushed the refactor/experimental-maps-slices branch from eac0f43 to feef255 Compare December 4, 2024 08:26
@wozniakjan
Copy link
Member

wozniakjan commented Dec 4, 2024

/run-e2e internal|opentelemetry_metrics
Update: You can check the progress here

@Juneezee
Copy link
Contributor Author

Juneezee commented Dec 4, 2024

I thought it might happen that we will need to bump go back to 1.23

https://github.com/kedacore/keda/actions/runs/12155644728/job/33897625757?pr=6372

pkg/scalers/scalersconfig/typed_config.go:206:18: slices.Sorted requires go1.23 or later (file is go1.22)
pkg/scalers/scalersconfig/typed_config.go:206:30: maps.Keys requires go1.23 or later (file is go1.22)
pkg/scalers/scalersconfig/typed_config.go:464:21: slices.Sorted requires go1.23 or later (file is go1.22)
pkg/scalers/scalersconfig/typed_config.go:464:33: maps.Keys requires go1.23 or later (file is go1.22)
make: *** [Makefile:147: vet] Error 1

@wozniakjan Thanks for correcting that for me 😊

@JorTurFer
Copy link
Member

This PR will revert the toolchain change of #6378 @wozniakjan , is it okey?

Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

Looking good, just a minor nit

go.mod Show resolved Hide resolved
@zroubalik zroubalik merged commit c8be1c7 into kedacore:main Dec 4, 2024
24 checks passed
@wozniakjan
Copy link
Member

This PR will revert the toolchain change of #6378 @wozniakjan , is it okey?

yeah, because at that point the toolchain and go versions are the same to support https://go.dev/doc/go1.23#iterators. I didn't realize this when approving #6378, but imho moving from experimental to core go packages has a higher value.

ctccxxd pushed a commit to ctccxxd/keda that referenced this pull request Dec 10, 2024
…ore#6372)

Signed-off-by: Eng Zer Jun <[email protected]>
Signed-off-by: Jan Wozniak <[email protected]>
Co-authored-by: Jan Wozniak <[email protected]>
@JorTurFer JorTurFer mentioned this pull request Dec 22, 2024
24 tasks
JorTurFer pushed a commit to JorTurFer/keda that referenced this pull request Dec 22, 2024
…ore#6372)

Signed-off-by: Eng Zer Jun <[email protected]>
Signed-off-by: Jan Wozniak <[email protected]>
Co-authored-by: Jan Wozniak <[email protected]>
JorTurFer pushed a commit to JorTurFer/keda that referenced this pull request Dec 22, 2024
…ore#6372)

Signed-off-by: Eng Zer Jun <[email protected]>
Signed-off-by: Jan Wozniak <[email protected]>
Co-authored-by: Jan Wozniak <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
JorTurFer added a commit that referenced this pull request Dec 24, 2024
* fix: CVE-2024-45337 (#6422)

Signed-off-by: Jorge Turrado <[email protected]>

* :fix: ensure consistent JSON log format for automaxprocs (#6335)

* fix: ensure consistent JSON log format for automaxprocs

Signed-off-by: Omer Aplatony <[email protected]>

* moved to Unreleased

Signed-off-by: Omer Aplatony <[email protected]>

---------

Signed-off-by: Omer Aplatony <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>

* Upgrade to the latest new-relic-go v2 client (#6325)

Signed-off-by: Jorge Turrado <[email protected]>

* refactor: replace experimental `maps` and `slices` with stdlib (#6372)

Signed-off-by: Eng Zer Jun <[email protected]>
Signed-off-by: Jan Wozniak <[email protected]>
Co-authored-by: Jan Wozniak <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>

* fix: Selenium Grid scaler exposes sum of pending and ongoing sessions to KDEA (#6368)

Signed-off-by: Jorge Turrado <[email protected]>

* fix: scaledjobs stuck as not ready (#6329)

since code was missing for setting a scaledjob as ready it was stuck as unready if there ever was a problem

This is a fix for a regression in #5916

Signed-off-by: Mårten Svantesson <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>

* fix: Paused ScaledObject count is reported correctly after operator restart (#6322)

Signed-off-by: Jorge Turrado <[email protected]>

* update changelog

Signed-off-by: Jorge Turrado <[email protected]>

* Bump go and deps (#6434)

* Bump go and deps

Signed-off-by: Jorge Turrado <[email protected]>

* bump golang.org/x/net

Signed-off-by: Jorge Turrado <[email protected]>

* Add missing permissions to code-scaners

Signed-off-by: Jorge Turrado <[email protected]>

* update release tracker

Signed-off-by: Jorge Turrado <[email protected]>

* fix aws scaler

Signed-off-by: Jorge Turrado <[email protected]>

* Pass theregion

Signed-off-by: Jorge Turrado <[email protected]>

* Pass theregion

Signed-off-by: Jorge Turrado <[email protected]>

* fix aws tests

Signed-off-by: Jorge Turrado <[email protected]>

* fix aws tests

Signed-off-by: Jorge Turrado <[email protected]>

* Update AWS admission probes

Signed-off-by: Jorge Turrado <[email protected]>

---------

Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>

* commit last vendored deps

Signed-off-by: Jorge Turrado <[email protected]>

* fix: Selenium Grid in case multiple scaler triggers are activate (#6437)

* fix: Selenium Grid scaler avoids overlapping when multiple browserVersion triggers are active

Signed-off-by: Viet Nguyen Duc <[email protected]>

* Update CHANGELOG

Signed-off-by: Viet Nguyen Duc <[email protected]>

* Fix e2e template test

Signed-off-by: Viet Nguyen Duc <[email protected]>

* Change imagePullPolicy to Always to take latest change

Signed-off-by: Viet Nguyen Duc <[email protected]>

* Update platformName default value as empty

Signed-off-by: Viet Nguyen Duc <[email protected]>

---------

Signed-off-by: Viet Nguyen Duc <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>

* add missing change

Signed-off-by: Jorge Turrado <[email protected]>

---------

Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Omer Aplatony <[email protected]>
Signed-off-by: Eng Zer Jun <[email protected]>
Signed-off-by: Jan Wozniak <[email protected]>
Signed-off-by: Mårten Svantesson <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Viet Nguyen Duc <[email protected]>
Co-authored-by: Omer Aplatony <[email protected]>
Co-authored-by: smcavallo <[email protected]>
Co-authored-by: Eng Zer Jun <[email protected]>
Co-authored-by: Jan Wozniak <[email protected]>
Co-authored-by: Viet Nguyen Duc <[email protected]>
Co-authored-by: Mårten Svantesson <[email protected]>
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