kgusakov commented on code in PR #905:
URL: https://github.com/apache/ignite-3/pull/905#discussion_r919491639


##########
modules/cli/src/integrationTest/java/org/apache/ignite/rest/ItGeneratedRestClientTest.java:
##########
@@ -81,7 +87,10 @@ public class ItGeneratedRestClientTest {
 
     private ClusterManagementApi clusterManagementApi;
 
+    private NodeManagementApi nodeManagementApi;
+
     private ObjectMapper objectMapper;
+    private String firstNodeName;

Review Comment:
   ```suggestion
   
       private String firstNodeName;
   ```



##########
modules/cli/src/main/java/org/apache/ignite/cli/call/cluster/status/ClusterStatusCall.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.cli.call.cluster.status;
+
+import jakarta.inject.Singleton;
+import 
org.apache.ignite.cli.call.cluster.status.ClusterStatus.ClusterStatusBuilder;
+import org.apache.ignite.cli.core.call.Call;
+import org.apache.ignite.cli.core.call.CallOutput;
+import org.apache.ignite.cli.core.call.DefaultCallOutput;
+import org.apache.ignite.cli.core.call.StatusCallInput;
+import org.apache.ignite.cli.core.exception.IgniteCliApiException;
+import org.apache.ignite.cli.deprecated.CliPathsConfigLoader;
+import org.apache.ignite.cli.deprecated.IgnitePaths;
+import org.apache.ignite.cli.deprecated.builtins.node.NodeManager;
+import org.apache.ignite.rest.client.api.ClusterManagementApi;
+import org.apache.ignite.rest.client.invoker.ApiClient;
+import org.apache.ignite.rest.client.invoker.ApiException;
+import org.apache.ignite.rest.client.model.ClusterState;
+
+/**
+ * Call to get cluster status.
+ */
+@Singleton
+public class ClusterStatusCall implements Call<StatusCallInput, ClusterStatus> 
{
+
+    private final NodeManager nodeManager;
+
+    private final CliPathsConfigLoader cliPathsCfgLdr;
+
+    /**
+     * Default constructor.
+     */
+    public ClusterStatusCall(NodeManager nodeManager, CliPathsConfigLoader 
cliPathsCfgLdr) {
+        this.nodeManager = nodeManager;
+        this.cliPathsCfgLdr = cliPathsCfgLdr;
+    }
+
+    @Override
+    public CallOutput<ClusterStatus> execute(StatusCallInput input) {
+        IgnitePaths paths = cliPathsCfgLdr.loadIgnitePathsOrThrowError();
+        ClusterStatusBuilder clusterStatusBuilder = ClusterStatus.builder()
+                .nodeCount(nodeManager.getRunningNodes(paths.logDir, 
paths.cliPidsDir()).size());
+
+        try {
+            ClusterState clusterState = 
fetchClusterState(input.getClusterUrl());
+            clusterStatusBuilder
+                    .initialized(true)
+                    .name(clusterState.getClusterTag().getClusterName())
+                    .metadataStorageNodes(clusterState.getMsNodes())
+                    .cmgNodes(clusterState.getCmgNodes());
+        } catch (ApiException e) {
+            if (e.getCode() == 404) { // NOT_FOUND means the cluster is not 
initialized yet
+                clusterStatusBuilder.initialized(false);
+            } else {
+                return DefaultCallOutput.failure(new IgniteCliApiException(e, 
input.getClusterUrl()));
+            }
+        } catch (IllegalArgumentException e) {
+            clusterStatusBuilder.initialized(false);

Review Comment:
   Not sure, that I understand, why IllegalArgumentException means, that 
cluster is not initialized? Could you write small comment?



##########
modules/node-management/pom.xml:
##########
@@ -0,0 +1,156 @@
+<?xml version="1.0" encoding="UTF-8"?>

Review Comment:
   It looks like it is not 'node-management' module, but just module with rest 
controller and some interfaces for it. Actual node management is doing under 
'runner' module. 
   If we create the separate top-level module for this kind of job - I fear we 
will be drowned in one-controller modules and become the REST factory instead 
of database :)
   
   I agree, that maybe the runner is not the best place for it, but at the 
moment it is the real place where node lifecycle manages.
   
   Maybe, if we have other controllers (which can't be placed in any existent 
modules), we'll should return to this topic and maybe create the separate place 
or package inside current 'rest*' modules. WDYT?



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

Reply via email to