You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by "wangao1236 (via GitHub)" <gi...@apache.org> on 2023/02/17 12:38:22 UTC
[GitHub] [incubator-uniffle] wangao1236 opened a new pull request, #629: [operator]: support specifying custom ports
wangao1236 opened a new pull request, #629:
URL: https://github.com/apache/incubator-uniffle/pull/629
<!--
1. Title: [#<issue>] <type>(<scope>): <subject>
Examples:
- "[#123] feat(operator): support xxx"
- "[#233] fix: check null before access result in xxx"
- "[MINOR] refactor: fix typo in variable name"
- "[MINOR] docs: fix typo in README"
- "[#255] test: fix flaky test NameOfTheTest"
Reference: https://www.conventionalcommits.org/en/v1.0.0/
2. Contributor guidelines:
https://github.com/apache/incubator-uniffle/blob/master/CONTRIBUTING.md
3. If the PR is unfinished, please mark this PR as draft.
-->
### What changes were proposed in this pull request?
Fix #627
(Please outline the changes and how this PR fixes the issue.)
### Why are the changes needed?
Fix #627.
### Does this PR introduce _any_ user-facing change?
For RSS cluster admin, they can set custom ports for shuffle servers and coordinators.
### How was this patch tested?
Manually verified.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on pull request #629: [#627] fix(operator): support specifying custom ports
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #629:
URL: https://github.com/apache/incubator-uniffle/pull/629#issuecomment-1436746862
> Should this pr be included in 0.7? @advancedxy
It's better to include this one since it's a bug fix. also cc @zuston
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on pull request #629: [#627] fix(operator): support specifying custom ports
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #629:
URL: https://github.com/apache/incubator-uniffle/pull/629#issuecomment-1436927887
Thanks @wangao1236, merging this. And if possible, would you cherry-pick this pr to branch-0.7?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on pull request #629: [#627] fix(operator): support specifying custom ports
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on PR #629:
URL: https://github.com/apache/incubator-uniffle/pull/629#issuecomment-1436942298
> Thanks @wangao1236, merging this. And if possible, would you cherry-pick this pr to branch-0.7?
If there are no conficts, you can commit directly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on pull request #629: [#627] fix(operator): support specifying custom ports
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on PR #629:
URL: https://github.com/apache/incubator-uniffle/pull/629#issuecomment-1436742566
Should this pr be included in 0.7? @advancedxy
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] wangao1236 commented on a diff in pull request #629: [#627] fix(operator): support specifying custom ports
Posted by "wangao1236 (via GitHub)" <gi...@apache.org>.
wangao1236 commented on code in PR #629:
URL: https://github.com/apache/incubator-uniffle/pull/629#discussion_r1111404673
##########
deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator.go:
##########
@@ -281,7 +281,7 @@ func GenerateAddresses(rss *unifflev1alpha1.RemoteShuffleService) string {
for i := 0; i < int(*rss.Spec.Coordinator.Count); i++ {
name := GenerateNameByIndex(rss, i)
serviceName := appendHeadless(name)
- current := fmt.Sprintf("%v:%v", serviceName, controllerconstants.ContainerShuffleServerRPCPort)
+ current := fmt.Sprintf("%v:%v", serviceName, *rss.Spec.Coordinator.RPCPort)
Review Comment:
Ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] zuston commented on pull request #629: [#627] fix(operator): support specifying custom ports
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on PR #629:
URL: https://github.com/apache/incubator-uniffle/pull/629#issuecomment-1436751540
> > Should this pr be included in 0.7? @advancedxy
>
> It's better to include this one since it's a bug fix. also cc @zuston
It's OK for me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy merged pull request #629: [#627] fix(operator): support specifying custom ports
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy merged PR #629:
URL: https://github.com/apache/incubator-uniffle/pull/629
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on a diff in pull request #629: [#627] fix(operator): support specifying custom ports
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #629:
URL: https://github.com/apache/incubator-uniffle/pull/629#discussion_r1109755464
##########
deploy/kubernetes/operator/pkg/controller/sync/coordinator/coordinator.go:
##########
@@ -281,7 +281,7 @@ func GenerateAddresses(rss *unifflev1alpha1.RemoteShuffleService) string {
for i := 0; i < int(*rss.Spec.Coordinator.Count); i++ {
name := GenerateNameByIndex(rss, i)
serviceName := appendHeadless(name)
- current := fmt.Sprintf("%v:%v", serviceName, controllerconstants.ContainerShuffleServerRPCPort)
+ current := fmt.Sprintf("%v:%v", serviceName, *rss.Spec.Coordinator.RPCPort)
Review Comment:
Could you also update the UT in the test file. Should add lines to verify the port is correct.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org