[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #1991: IGNITE-19077 Apply custom cluster config on cluster init

["PakhomovAlexander (via GitHub)" <gi...@apache.org>]

PakhomovAlexander commented on code in PR #1991:
URL: https://github.com/apache/ignite-3/pull/1991#discussion_r1186055418


##########
modules/cli/src/integrationTest/java/org/apache/ignite/internal/rest/ItGeneratedRestClientTest.java:
##########
@@ -323,7 +322,7 @@ void initClusterNoSuchNode() throws JsonProcessingException {
                                 .clusterName("cluster")
                                 .metaStorageNodes(List.of("no-such-node"))
                                 .cmgNodes(List.of())
-                                .authenticationConfig(new AuthenticationConfig().enabled(false)))
+                                .clusterConfiguration(null))

Review Comment:
   Can you just skip this call? 



##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/ClusterInitializer.java:
##########
@@ -83,14 +81,14 @@ public CompletableFuture<Void> initCluster(
      * @param cmgNodeNames Names of nodes that will host the Cluster Management Group. Can be empty, in which case {@code
      * metaStorageNodeNames} will be used instead.
      * @param clusterName Human-readable name of the cluster.
-     * @param authenticationConfig REST authentication configuration.
+     * @param clusterConfiguration Cluster configuration.
      * @return Future that represents the state of the operation.
      */
     public CompletableFuture<Void> initCluster(
             Collection<String> metaStorageNodeNames,
             Collection<String> cmgNodeNames,
             String clusterName,
-            AuthenticationConfig authenticationConfig
+            @Nullable String clusterConfiguration

Review Comment:
   Can you add one more method `initCluster ` that will call this one with `clusterConfiguration = null`?



##########
modules/cli/src/integrationTest/java/org/apache/ignite/internal/rest/ItGeneratedRestClientTest.java:
##########
@@ -298,7 +297,7 @@ void initCluster() {
                             .clusterName("cluster")
                             .metaStorageNodes(List.of(firstNodeName))
                             .cmgNodes(List.of())
-                            .authenticationConfig(new AuthenticationConfig().enabled(false))
+                            .clusterConfiguration(null)

Review Comment:
   Can be skipped



##########
modules/rest-api/openapi/openapi.yaml:
##########
@@ -833,11 +797,9 @@ components:
         clusterName:
           type: string
           description: The name of the cluster.
-        authenticationConfig:
-          allOf:
-          - $ref: '#/components/schemas/AuthenticationConfig'
-          - type: object
-            description: Authentication configuration.
+        clusterConfiguration:
+          type: string
+          description: Cluster configuration.

Review Comment:
   Could you adjust the description? I think it would be useful to mention the HOCON format of the provided string.



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org