-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
(#1982) Publish Spark Operator image for multiple Spark versions on r… #2024
base: master
Are you sure you want to change the base?
(#1982) Publish Spark Operator image for multiple Spark versions on r… #2024
Conversation
…ons on release Signed-off-by: Peter Jablonski <mcclonski.peter@gmail.com> Signed-off-by: Peter McClonski <mcclonski.peter@gmail.com>
@@ -56,6 +56,10 @@ jobs: | |||
strategy: | |||
fail-fast: false | |||
matrix: | |||
spark_version: | |||
- 3.5.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like repeating this between the two jobs, but it was the least complex solution.
version: 1.2.15 | ||
appVersion: v1beta2-1.4.6-3.5.0 | ||
version: 1.2.16 | ||
appVersion: v1beta2-1.4.6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I: The Spark version is separate from the Operator version. New image tag format is consistent with the old one, but with parameterized Spark version rather than hardcoding it here.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vara-bonthu The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@peter-mcclonski Please resolve conflicts. Will review and merge the PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR itself looks good. But I wonder if there really is a use case for building a separate image for each version. @peter-mcclonski Have you run into cases where, for example, operator built with Spark 3.4.3 and 3.5.1 result in difference behaviors?
I actually have never tried it-- internally, we've always made a habit of aligning our Operator version to whatever version of spark we're using. If it doesn't actually need to be aligned (which I recall you mentioning in the first community call), then you're right, this may not be a value-add. |
Let's keep this open and see if users show interest in this feature. If we receive more thumbs up, we can maintain it; otherwise, we may consider de-scoping it. |
This is great! @peter-mcclonski I think we should update the version matrix here: https://github.com/kubeflow/spark-operator?tab=readme-ov-file#version-matrix |
…elease
🛑 Important:
Please open an issue to discuss significant work before you start. We appreciate your contributions and don't want your efforts to go to waste!
For guidelines on how to contribute, please review the CONTRIBUTING.md document.
Purpose of this PR
Support multiple versions of Spark with our release process.
Proposed changes:
Change Category
Indicate the type of change by marking the applicable boxes:
Rationale
See #1982
Checklist
Before submitting your PR, please review the following:
Additional Notes
Not sure of the best way to validate this action works prior to merging. Would be eager for feedback on testing it.