FMX commented on code in PR #3323:
URL: https://github.com/apache/celeborn/pull/3323#discussion_r2149472835
##########
cli/src/main/scala/org/apache/celeborn/cli/master/MasterSubcommandImpl.scala:
##########
@@ -212,6 +214,51 @@ class MasterSubcommandImpl extends MasterSubcommand {
commonOptions.configName,
commonOptions.getAuthHeader)
+ private[master] def runUpsertDynamicConf: HandleResponse = {
Review Comment:
The following code block is identical to the worker's subcommand
implementation. These codes can be reused.
##########
cli/src/main/scala/org/apache/celeborn/cli/master/MasterSubcommandImpl.scala:
##########
@@ -212,6 +214,51 @@ class MasterSubcommandImpl extends MasterSubcommand {
commonOptions.configName,
commonOptions.getAuthHeader)
+ private[master] def runUpsertDynamicConf: HandleResponse = {
Review Comment:
Maybe you can extract a common command.
##########
cli/src/test/scala/org/apache/celeborn/cli/TestCelebornCliCommands.scala:
##########
@@ -230,6 +252,28 @@ class TestCelebornCliCommands extends CelebornFunSuite
with MiniClusterFeature {
captureOutputAndValidateResponse(args, "")
}
+ test("master --upsert-dynamic-conf") {
+ cancel("This test is temporarily disabled since dynamic conf is not
enabled in unit tests.")
+ val args = prepareWorkerArgs() ++ Array(
Review Comment:
It should be the `prepareMasterArgs`.
##########
openapi/openapi-client/src/main/openapi3/master_rest_v1.yaml:
##########
@@ -93,6 +93,58 @@ paths:
"503":
description: Dynamic configuration is disabled.
+ /api/v1/conf/dynamic/upsert:
Review Comment:
The following code block is identical to the worker's. You can refer to this
link to reuse
them(https://dev.to/mikeralphson/defining-reusable-components-with-the-openapi-specification-4077).
##########
cli/src/test/scala/org/apache/celeborn/cli/TestCelebornCliCommands.scala:
##########
@@ -230,6 +252,28 @@ class TestCelebornCliCommands extends CelebornFunSuite
with MiniClusterFeature {
captureOutputAndValidateResponse(args, "")
}
+ test("master --upsert-dynamic-conf") {
+ cancel("This test is temporarily disabled since dynamic conf is not
enabled in unit tests.")
+ val args = prepareWorkerArgs() ++ Array(
+ "--upsert-dynamic-conf",
+ "--config-level",
+ "SYSTEM",
+ "--upsert-configs",
+ "key1:val1,key2:val2")
+ captureOutputAndValidateResponse(args, "success: true")
+ }
+
+ test("master --delete-dynamic-conf") {
+ cancel("This test is temporarily disabled since dynamic conf is not
enabled in unit tests.")
+ val args = prepareWorkerArgs() ++ Array(
Review Comment:
Ditto.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
