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

Update Golang to 1.23 and Spark to 3.5.2 #2150

Closed

Conversation

jacobsalway
Copy link
Member

@jacobsalway jacobsalway commented Sep 3, 2024

Purpose

Upgrade Golang from 1.22.5 to 1.23 and Spark from 3.5.0 to 3.5.2, which includes both the Dockerfile and the examples.

Testing

Screenshot 2024-09-04 at 21 51 57

Screenshot 2024-09-04 at 21 57 02

Change Category

Indicate the type of change by marking the applicable boxes:

  • Bugfix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that could affect existing functionality)
  • Documentation update

Checklist

Before submitting your PR, please review the following:

  • I have conducted a self-review of my own code.
  • I have updated documentation accordingly.
  • I have added tests that prove my changes are effective or that my feature works.
  • Existing unit tests pass locally with my changes.

Signed-off-by: Jacob Salway <jacob.salway@gmail.com>
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign yuchaoran2011 for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jacobsalway jacobsalway marked this pull request as ready for review September 4, 2024 12:02
go.mod Outdated
@@ -1,6 +1,6 @@
module github.com/kubeflow/spark-operator

go 1.22.5
go 1.23.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need the minor version or can we use simply 1.23?

Copy link
Member Author

Choose a reason for hiding this comment

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

Definitely not strictly necessary given Go's backward compatibility guarantees and not pinning should reduce the frequency of toolchain upgrade PRs. Updated

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the patch version is needed, otherwise go mod tidy will fail:

$ go mod tidy
go: downloading go1.23 (darwin/arm64)
go: download go1.23 for darwin/arm64: toolchain not available

There are some issues related to the toolchain version not being available: golang/go#62278.

Signed-off-by: Jacob Salway <jacob.salway@gmail.com>
@jacobsalway jacobsalway force-pushed the chore/update-dependencies branch from 857110f to 6f7336b Compare September 9, 2024 00:33
@jacobsalway jacobsalway changed the title Update Golang to 1.23.0 and Spark to 3.5.2 Update Golang to 1.23 and Spark to 3.5.2 Sep 9, 2024
@ChenYi015
Copy link
Contributor

@jacobsalway Could we have two dedicated PRs, one for bumping Golang version to 1.23.0, and another for bumping Spark version to 3.5.2.

@ChenYi015
Copy link
Contributor

We should merge #2149 first so that the e2e test CI can pass.

@jacobsalway
Copy link
Member Author

@ChenYi015 you'll need to get an approver to add the label on the other PR since I only have review permission

@ChenYi015
Copy link
Contributor

@ChenYi015 you'll need to get an approver to add the label on the other PR since I only have review permission

@jacobsalway I had approved the e2e fix PR myself.

@jacobsalway
Copy link
Member Author

Didn't realise the bot allowed you to approve your own PR 🤔

Splitting this PR into #2155 and #2154

@jacobsalway jacobsalway closed this Sep 9, 2024
@jacobsalway jacobsalway deleted the chore/update-dependencies branch September 9, 2024 03:20
@ChenYi015
Copy link
Contributor

Didn't realise the bot allowed you to approve your own PR

I checked out the community guide and found that I can approve my PR, but I cannot lgtm my PR.

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

Successfully merging this pull request may close these issues.

3 participants