-
Notifications
You must be signed in to change notification settings - Fork 700
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
Upgrade to go 1.23 #6249
base: master
Are you sure you want to change the base?
Upgrade to go 1.23 #6249
Conversation
Code Review Agent Run Status
|
429e8a1
to
72b75ca
Compare
Code Review Agent Run Status
|
A little gift to help with this: our fork of mockery has just been removed. |
Much appreciated thank you |
Code Review Agent Run Status
|
Code Review Agent Run Status
|
Signed-off-by: Jason Parraga <[email protected]>
Signed-off-by: Jason Parraga <[email protected]>
Code Review Agent Run #6f4da4Actionable Suggestions - 9
Additional Suggestions - 10
Review Details
|
Changelist by BitoThis pull request implements the following key changes.
|
flytepropeller/pkg/apis/flyteworkflow/v1alpha1/mocks/MutableNodeStatus.go
Show resolved
Hide resolved
flytepropeller/pkg/apis/flyteworkflow/v1alpha1/mocks/MutableNodeStatus.go
Show resolved
Hide resolved
flytepropeller/pkg/apis/flyteworkflow/v1alpha1/mocks/ExecutableNodeStatus.go
Show resolved
Hide resolved
flytepropeller/pkg/apis/flyteworkflow/v1alpha1/mocks/MutableWorkflowNodeStatus.go
Show resolved
Hide resolved
Can we separate the move to go 1.23 from the bump in mockery version? Since moving to this new version is non-controversial, how about we do it separately (either make the go1.23 changes happen in a stacked PR or, my preference, move to that mockery version in a separate PR and merge that sooner). |
Sure |
So I'm having trouble because it seems that the mockery libraries are tightly coupled with a very specific version of go. When I use the version of mockery that supports go 1.23 I'm running into this error like `2025/03/03 20:30:44 internal error: package "context" without types was imported from "github.com/flyteorg/flyte/flytestdlib/utils". I even tried the earliest version that supports the go 1.23 type aliasing, v2.46.1. ie: https://vektra.github.io/mockery/latest/notes/#internal-error-package-without-types-was-imported |
Hmm. I can't even run |
Alright I can't really land the mockery changes in a separate PR since newer versions of mockery require new versions of go.
I messed around with it for a few hours and different versions of mockery all have different issues but it seems like all the issues are rooted in the fact that you use a different version of go to generate the mocks then you do to use them. |
Why are the changes needed?
Keeping Go up to date is beneficial since new versions have improvements for common source of bugs/issues as well as performance improvements and new features.
This change will also allow us to move to go 1.24 when that it deemed stable enough.
What changes were proposed in this pull request?
How was this patch tested?
Unit tests
Check all the applicable boxes
Related PRs
Docs link
Summary by Bito
This PR upgrades the Go version from 1.22 to 1.23 across all Flyte components and their dependencies. The changes include updating base Docker images, upgrading mockery to v2.52.1 to support Go 1.23's new alias type feature, and updating various core Go dependencies. All mock files have been regenerated with the new mockery version, resulting in updated function signatures and improved compatibility.Unit tests added: False
Estimated effort to review (1-5, lower is better): 5