You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "akpatnam25 (via GitHub)" <gi...@apache.org> on 2024/09/12 07:28:15 UTC

[PR] [CELEBORN-1593][FOLLOWUP] Add CLI support for removeWorkersUnavailabeInfo [celeborn]

akpatnam25 opened a new pull request, #2735:
URL: https://github.com/apache/celeborn/pull/2735

   <!--
   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?
   Adding support for `removeWorkersUnavailabeInfo. This was missed in the last PR. 
   
   
   ### Why are the changes needed?
   see above
   
   
   ### Does this PR introduce _any_ user-facing change?
   no
   
   
   ### How was this patch tested?
   added unit test
   


-- 
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

Re: [PR] [CELEBORN-1593][FOLLOWUP] Add CLI support for removeWorkersUnavailabeInfo [celeborn]

Posted by "RexXiong (via GitHub)" <gi...@apache.org>.
RexXiong commented on code in PR #2735:
URL: https://github.com/apache/celeborn/pull/2735#discussion_r1759661735


##########
cli/src/main/scala/org/apache/celeborn/cli/master/MasterSubcommandImpl.scala:
##########
@@ -77,6 +78,14 @@ class MasterSubcommandImpl extends Runnable with MasterSubcommand {
     workerApi.excludeWorker(removeExcludeWorkerRequest)
   }
 
+  private[master] def runRemoveWorkersUnavailableInfo: HandleResponse = {
+    val workerIds = getWorkerIds
+    val removeWorkersUnavailableInfoRequest =
+      new RemoveWorkersUnavailableInfoRequest().workers(workerIds)
+    logInfo(s"Sending remove workers unavailable info requests to workers: $workerIds")

Review Comment:
   ```suggestion
       logInfo(s"Sending remove workers unavailable info requests to master: $workerIds")
   ```



-- 
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

Re: [PR] [CELEBORN-1593][FOLLOWUP] Add CLI support for some missing HTTP APIs [celeborn]

Posted by "akpatnam25 (via GitHub)" <gi...@apache.org>.
akpatnam25 commented on PR #2735:
URL: https://github.com/apache/celeborn/pull/2735#issuecomment-2354655300

   ping @SteNicholas @RexXiong @FMX, thanks :) 


-- 
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

Re: [PR] [CELEBORN-1593][FOLLOWUP] Add CLI support for some missing HTTP APIs [celeborn]

Posted by "akpatnam25 (via GitHub)" <gi...@apache.org>.
akpatnam25 commented on code in PR #2735:
URL: https://github.com/apache/celeborn/pull/2735#discussion_r1764548965


##########
LICENSE-binary:
##########
@@ -309,6 +309,7 @@ org.slf4j:jcl-over-slf4j
 org.webjars:swagger-ui
 org.xerial.snappy:snappy-java
 org.yaml:snakeyaml
+info.picocli:picocli

Review Comment:
   sure updated



-- 
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

Re: [PR] [CELEBORN-1593][FOLLOWUP] Add CLI support for some missing HTTP APIs [celeborn]

Posted by "FMX (via GitHub)" <gi...@apache.org>.
FMX commented on PR #2735:
URL: https://github.com/apache/celeborn/pull/2735#issuecomment-2357729909

   Thanks. Merged into main(v0.6.0).


-- 
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

Re: [PR] [CELEBORN-1593][FOLLOWUP] Add CLI support for some missing HTTP APIs [celeborn]

Posted by "FMX (via GitHub)" <gi...@apache.org>.
FMX closed pull request #2735: [CELEBORN-1593][FOLLOWUP] Add CLI support for some missing HTTP APIs
URL: https://github.com/apache/celeborn/pull/2735


-- 
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

Re: [PR] [CELEBORN-1593][FOLLOWUP] Add CLI support for removeWorkersUnavailabeInfo [celeborn]

Posted by "waitinfuture (via GitHub)" <gi...@apache.org>.
waitinfuture commented on PR #2735:
URL: https://github.com/apache/celeborn/pull/2735#issuecomment-2345738533

   Hi @akpatnam25 , could you add your email to your github account so that github can track your commits?


-- 
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

Re: [PR] [CELEBORN-1593][FOLLOWUP] Add CLI support for some missing HTTP APIs [celeborn]

Posted by "akpatnam25 (via GitHub)" <gi...@apache.org>.
akpatnam25 commented on PR #2735:
URL: https://github.com/apache/celeborn/pull/2735#issuecomment-2357721993

   thanks, updated @FMX 


-- 
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

Re: [PR] [CELEBORN-1593][FOLLOWUP] Add CLI support for some missing HTTP APIs [celeborn]

Posted by "FMX (via GitHub)" <gi...@apache.org>.
FMX commented on code in PR #2735:
URL: https://github.com/apache/celeborn/pull/2735#discussion_r1764508865


##########
LICENSE-binary:
##########
@@ -309,6 +309,7 @@ org.slf4j:jcl-over-slf4j
 org.webjars:swagger-ui
 org.xerial.snappy:snappy-java
 org.yaml:snakeyaml
+info.picocli:picocli

Review Comment:
   Please keep this license files sorted in alphabetical order.



-- 
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

Re: [PR] [CELEBORN-1593][FOLLOWUP] Add CLI support for removeWorkersUnavailabeInfo [celeborn]

Posted by "akpatnam25 (via GitHub)" <gi...@apache.org>.
akpatnam25 commented on PR #2735:
URL: https://github.com/apache/celeborn/pull/2735#issuecomment-2345496648

   ping @SteNicholas @turboFei @FMX 


-- 
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

Re: [PR] [CELEBORN-1593][FOLLOWUP] Add CLI support for removeWorkersUnavailabeInfo [celeborn]

Posted by "akpatnam25 (via GitHub)" <gi...@apache.org>.
akpatnam25 commented on PR #2735:
URL: https://github.com/apache/celeborn/pull/2735#issuecomment-2346901943

   @waitinfuture I think it seems to be taking my work email, which I am not allowed to add as an email. From now on before committing, I can run: `git config user.email` with my verified email address.  I amended the previous commits in this PR to be the correct email now: 
   ```
   apatnam-mn3092:incubator-celeborn apatnam$ git log
   commit 57f1c6bad035f7a2dd334d83c745c507e6ae1054 (HEAD -> CELEBORN-1593, origin/CELEBORN-1593)
   Author: Aravind Patnam <ak...@gmail.com>
   Date:   Thu Sep 12 00:32:26 2024 -0700
   
       spotless
   
   commit f17c06691fee76ab5766f40222c8e935ba866434
   Author: Aravind Patnam <ak...@gmail.com>
   Date:   Thu Sep 12 00:27:04 2024 -0700
   
       [CELEBORN-1593][FOLLOWUP] Add CLI support for removeWorkersUnavailableInfo
   
   ```


-- 
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

Re: [PR] [CELEBORN-1593][FOLLOWUP] Add CLI support for removeWorkersUnavailabeInfo [celeborn]

Posted by "akpatnam25 (via GitHub)" <gi...@apache.org>.
akpatnam25 commented on PR #2735:
URL: https://github.com/apache/celeborn/pull/2735#issuecomment-2346914202

   > LGTM. @akpatnam25, btw, do all REST API have the corresponding cli command? It's better to support other missing command in this pull request.
   
   @SteNicholas yes, this was the only one I missed since this API was added a bit later. 


-- 
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

Re: [PR] [CELEBORN-1593][FOLLOWUP] Add CLI support for some missing HTTP APIs [celeborn]

Posted by "akpatnam25 (via GitHub)" <gi...@apache.org>.
akpatnam25 commented on PR #2735:
URL: https://github.com/apache/celeborn/pull/2735#issuecomment-2350887385

   > LGTM. @akpatnam25, btw, do all REST API have the corresponding cli command? It's better to support other missing command in this pull request.
   
   the missing APIs should be updated now in this PR. 


-- 
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