You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "pan3793 (via GitHub)" <gi...@apache.org> on 2023/06/26 12:08:05 UTC
[GitHub] [incubator-celeborn] pan3793 opened a new pull request, #1624: [CELEBORN-715] Change master URL schema from rss to celeborn
pan3793 opened a new pull request, #1624:
URL: https://github.com/apache/incubator-celeborn/pull/1624
<!--
Thanks for sending a pull request! Here are some tips for you:
- Make sure the PR title start w/ a JIRA ticket, e.g. '[CELEBORN-XXXX] Your PR title ...'.
- Be sure to keep the PR description updated to reflect all changes.
- Please write your PR title to summarize what this PR proposes.
- If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
Change Celeborn Master URL from `rss://<host>:port` to `celeborn://<host>:port`
### Why are the changes needed?
Respect the project name.
### Does this PR introduce _any_ user-facing change?
No, it's the internal change.
### How was this patch tested?
Pass GA.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] waitinfuture closed pull request #1624: [CELEBORN-715] Change master URL schema from rss to celeborn
Posted by "waitinfuture (via GitHub)" <gi...@apache.org>.
waitinfuture closed pull request #1624: [CELEBORN-715] Change master URL schema from rss to celeborn
URL: https://github.com/apache/incubator-celeborn/pull/1624
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] waitinfuture commented on pull request #1624: [CELEBORN-715] Change master URL schema from rss to celeborn
Posted by "waitinfuture (via GitHub)" <gi...@apache.org>.
waitinfuture commented on PR #1624:
URL: https://github.com/apache/incubator-celeborn/pull/1624#issuecomment-1607410992
> Add migration guide then?
Yeah, I think we need to doc this.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] pan3793 commented on pull request #1624: [CELEBORN-715] Change master URL schema from rss to celeborn
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #1624:
URL: https://github.com/apache/incubator-celeborn/pull/1624#issuecomment-1607499764
@waitinfuture FYI, the migration guide is added, please take another look.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] pan3793 commented on pull request #1624: [CELEBORN-715] Change master URL schema from rss to celeborn
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #1624:
URL: https://github.com/apache/incubator-celeborn/pull/1624#issuecomment-1607408686
Add migration guide then?
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] pan3793 commented on a diff in pull request #1624: [CELEBORN-715] Change master URL schema from rss to celeborn
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #1624:
URL: https://github.com/apache/incubator-celeborn/pull/1624#discussion_r1242093484
##########
common/src/main/scala/org/apache/celeborn/common/util/Utils.scala:
##########
@@ -298,15 +284,14 @@ object Utils extends Logging {
(base + offset - 1024) % (65536 - 1024) + 1024
}
+ @tailrec
def isBindCollision(exception: Throwable): Boolean = {
exception match {
case e: BindException =>
if (e.getMessage != null) {
return true
}
isBindCollision(e.getCause)
- // case e: MultiException =>
Review Comment:
for jetty case, we don't need it
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] waitinfuture commented on pull request #1624: [CELEBORN-715] Change master URL schema from rss to celeborn
Posted by "waitinfuture (via GitHub)" <gi...@apache.org>.
waitinfuture commented on PR #1624:
URL: https://github.com/apache/incubator-celeborn/pull/1624#issuecomment-1607400540
Currently users can start worker with ./sbin/start-worker.sh rss://${masterhost}:${masterport}, so seems this breaks the user interface.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] pan3793 commented on a diff in pull request #1624: [CELEBORN-715] Change master URL schema from rss to celeborn
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #1624:
URL: https://github.com/apache/incubator-celeborn/pull/1624#discussion_r1242094939
##########
common/src/main/scala/org/apache/celeborn/common/util/Utils.scala:
##########
@@ -52,8 +53,6 @@ import org.apache.celeborn.common.protocol.message.ControlMessages.WorkerResourc
object Utils extends Logging {
- def createDateFormat: SimpleDateFormat = new SimpleDateFormat("yyyyMMddHHmmss", Locale.US)
Review Comment:
useless, and `SimpleDateFormat` is discourage to use
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] pan3793 commented on a diff in pull request #1624: [CELEBORN-715] Change master URL schema from rss to celeborn
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #1624:
URL: https://github.com/apache/incubator-celeborn/pull/1624#discussion_r1242093192
##########
common/src/main/scala/org/apache/celeborn/common/util/Utils.scala:
##########
@@ -199,41 +220,6 @@ object Utils extends Logging {
}
}
- def tryWithSafeFinallyAndFailureCallbacks[T](block: => T)(
Review Comment:
useless
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] codecov[bot] commented on pull request #1624: [CELEBORN-715] Change master URL schema from rss to celeborn
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #1624:
URL: https://github.com/apache/incubator-celeborn/pull/1624#issuecomment-1607352823
## [Codecov](https://app.codecov.io/gh/apache/incubator-celeborn/pull/1624?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#1624](https://app.codecov.io/gh/apache/incubator-celeborn/pull/1624?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (86c2afa) into [main](https://app.codecov.io/gh/apache/incubator-celeborn/commit/6b82ecdfa0869c76c8473023aabc37a48b2433d7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (6b82ecd) will **decrease** coverage by `0.05%`.
> The diff coverage is `7.15%`.
> :exclamation: Current head 86c2afa differs from pull request most recent head 6c534b3. Consider uploading reports for the commit 6c534b3 to get more accurate results
```diff
@@ Coverage Diff @@
## main #1624 +/- ##
==========================================
- Coverage 45.92% 45.87% -0.05%
==========================================
Files 159 159
Lines 9935 9919 -16
Branches 970 969 -1
==========================================
- Hits 4562 4549 -13
+ Misses 5075 5069 -6
- Partials 298 301 +3
```
| [Impacted Files](https://app.codecov.io/gh/apache/incubator-celeborn/pull/1624?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...la/org/apache/celeborn/common/rpc/RpcAddress.scala](https://app.codecov.io/gh/apache/incubator-celeborn/pull/1624?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29tbW9uL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvY2VsZWJvcm4vY29tbW9uL3JwYy9ScGNBZGRyZXNzLnNjYWxh) | `25.00% <0.00%> (ø)` | |
| [.../scala/org/apache/celeborn/common/util/Utils.scala](https://app.codecov.io/gh/apache/incubator-celeborn/pull/1624?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29tbW9uL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvY2VsZWJvcm4vY29tbW9uL3V0aWwvVXRpbHMuc2NhbGE=) | `17.70% <0.00%> (+0.15%)` | :arrow_up: |
| [...pache/celeborn/common/rpc/RpcEndpointAddress.scala](https://app.codecov.io/gh/apache/incubator-celeborn/pull/1624?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29tbW9uL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvY2VsZWJvcm4vY29tbW9uL3JwYy9ScGNFbmRwb2ludEFkZHJlc3Muc2NhbGE=) | `53.85% <40.00%> (+25.85%)` | :arrow_up: |
... and [3 files with indirect coverage changes](https://app.codecov.io/gh/apache/incubator-celeborn/pull/1624/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org