-
Notifications
You must be signed in to change notification settings - Fork 458
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
[CELEBORN] Support celeborn 0.5.0 #6264
Conversation
Thanks for opening a pull request! Could you open an issue for this pull request on Github Issues? https://github.com/apache/incubator-gluten/issues Then could you also rename commit message and pull request title in the following format?
See also: |
Run Gluten Clickhouse CI |
Run Gluten Clickhouse CI |
Run Gluten Clickhouse CI |
1 similar comment
Run Gluten Clickhouse CI |
Run Gluten Clickhouse CI |
Ping @PHILO-HE @kerwin-zk |
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.
Looks good to me. Just one small suggestion. Please check whether it makes sense. Thanks!
@@ -532,7 +532,7 @@ jobs: | |||
fail-fast: false | |||
matrix: | |||
spark: [ "spark-3.2" ] | |||
celeborn: [ "celeborn-0.4.1", "celeborn-0.3.2-incubating" ] | |||
celeborn: [ "celeborn-0.5.0", "celeborn-0.4.1", "celeborn-0.3.2-incubating" ] |
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.
Nit:
Suggest to change it to [ "celeborn-0.5", "celeborn-0.4", "celeborn-0.3" ]
, consistent with profile name. Maybe, need to add celeborn-0.3
profile in pom.
Then, EXTRA_PROFILE="-P${{ matrix.celeborn }}"
and let pom determine which specific version will be used.
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 workflow requires a download link based on the specific version to deploy the celeborn service, then we should have to use the version instead of profile id.
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.
@yikf, I see. Thanks!
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.
LGTM.Thanks!
thanks @PHILO-HE @kerwin-zk |
What changes were proposed in this pull request?
Apache celeborn was released on June 24th, you can see also release notes for more details. This PR amins to support celeborn 0.5.0.
How was this patch tested?
GA it tests