You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2020/01/07 17:38:22 UTC

[GitHub] [helix] alirezazamani opened a new pull request #675: Add REST API for Cluster Creation with CloudConfig

alirezazamani opened a new pull request #675: Add REST API for Cluster Creation with CloudConfig
URL: https://github.com/apache/helix/pull/675
 
 
   ### Issues
   - [x] My PR addresses the following Helix issues and references them in the PR title:
   Fixes #674
   
   ### Description
   - [x] Here are some details about my PR, including screenshots of any UI changes:
   This PR contains the relevant REST API calls for cluster creation.
   This change allows user to create cluster with CloudConfig.
   Multiple tests have been added in order to check the functionality of REST calls.
   
   
   ### Tests
   
   - [x] The following is the result of the "mvn test" command on the appropriate module:
   New tests have been added.
   TestCloudAccessor.testAddCloudConfigNonExistedCluster
   TestCloudAccessor.testAddCloudConfig
   TestCloudAccessor.testDeleteCloudConfig
   TestCloudAccessor.testUpdateCloudConfig
   TestClusterAccessor.testAddClusterWithCloudConfig
   TestClusterAccessor.testAddClusterWithInvalidCloudConfig
   TestClusterAccessor.testAddClusterWithInvalidCustomizedCloudConfig
   TestClusterAccessor.testAddClusterWithValidCustomizedCloudConfig
   TestClusterAccessor.testAddClusterWithCloudConfigDisabledCloud
   
   Test Result 1: helix-core: mvn test
   [INFO] Tests run: 904, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 3,444.069 s - in TestSuite
   [INFO] 
   [INFO] Results:
   [INFO] 
   [INFO] Tests run: 904, Failures: 0, Errors: 0, Skipped: 0
   [INFO] 
   [INFO] ------------------------------------------------------------------------
   [INFO] BUILD SUCCESS
   [INFO] ------------------------------------------------------------------------
   [INFO] Total time:  57:29 min
   [INFO] Finished at: 2020-01-06T17:05:55-08:00
   [INFO] ------------------------------------------------------------------------
   
   Test Result 2: helix-rest: mvn test
   [INFO] Tests run: 99, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 26.832 s - in TestSuite
   [INFO] 
   [INFO] Results:
   [INFO] 
   [INFO] Tests run: 99, Failures: 0, Errors: 0, Skipped: 0
   [INFO] 
   [INFO] ------------------------------------------------------------------------
   [INFO] BUILD SUCCESS
   [INFO] ------------------------------------------------------------------------
   [INFO] Total time:  32.481 s
   [INFO] Finished at: 2020-01-07T09:25:20-08:00
   [INFO] ------------------------------------------------------------------------
   
   ### Commits
   
   - [x] My commits all reference appropriate Apache Helix GitHub issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
     1. Subject is separated from body by a blank line
     1. Subject is limited to 50 characters (not including Jira issue reference)
     1. Subject does not end with a period
     1. Subject uses the imperative mood ("add", not "adding")
     1. Body wraps at 72 characters
     1. Body explains "what" and "why", not "how"
   
   ### Code Quality
   
   - [x] My diff has been formatted using helix-style.xml

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] dasahcc commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig

Posted by GitBox <gi...@apache.org>.
dasahcc commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig
URL: https://github.com/apache/helix/pull/675#discussion_r363897194
 
 

 ##########
 File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/CloudAccessor.java
 ##########
 @@ -0,0 +1,152 @@
+package org.apache.helix.rest.server.resources.helix;
+
+/*
+ * 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.
+ */
+
+import java.io.IOException;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.Response;
+import org.apache.helix.ConfigAccessor;
+import org.apache.helix.HelixAdmin;
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.manager.zk.ZKUtil;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.CloudConfig;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.helix.model.builder.HelixConfigScopeBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Path("/clusters")
+public class CloudAccessor extends AbstractHelixResource {
+  private static Logger _logger = LoggerFactory.getLogger(CloudAccessor.class.getName());
+
+  @PUT
+  @Path("{clusterId}/cloudconfig")
+  public Response addCloudConfig(@PathParam("clusterId") String clusterId, String content) {
+
+    HelixZkClient zkClient = getHelixZkClient();
+    if (!ZKUtil.isClusterSetup(clusterId, zkClient)) {
+      return notFound();
+    }
+
+    HelixAdmin admin = getHelixAdmin();
+    ZNRecord record;
+    try {
+      record = toZNRecord(content);
+    } catch (IOException e) {
+      _logger.error("Failed to deserialize user's input " + content + ", Exception: " + e);
+      return badRequest("Input is not a vaild ZNRecord!");
+    }
 
 Review comment:
   Let's make it as a general function in AbstractResourceAccessor since it has been widely used in the code.
   
   Same as following places

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] alirezazamani commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig

Posted by GitBox <gi...@apache.org>.
alirezazamani commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig
URL: https://github.com/apache/helix/pull/675#discussion_r364398816
 
 

 ##########
 File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/CloudAccessor.java
 ##########
 @@ -0,0 +1,152 @@
+package org.apache.helix.rest.server.resources.helix;
+
+/*
+ * 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.
+ */
+
+import java.io.IOException;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.Response;
+import org.apache.helix.ConfigAccessor;
+import org.apache.helix.HelixAdmin;
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.manager.zk.ZKUtil;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.CloudConfig;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.helix.model.builder.HelixConfigScopeBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Path("/clusters")
 
 Review comment:
   Fixed. Move to code to ClusterAccessor.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] alirezazamani commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig

Posted by GitBox <gi...@apache.org>.
alirezazamani commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig
URL: https://github.com/apache/helix/pull/675#discussion_r366615050
 
 

 ##########
 File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/CloudAccessor.java
 ##########
 @@ -0,0 +1,152 @@
+package org.apache.helix.rest.server.resources.helix;
+
+/*
+ * 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.
+ */
+
+import java.io.IOException;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.Response;
+import org.apache.helix.ConfigAccessor;
+import org.apache.helix.HelixAdmin;
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.manager.zk.ZKUtil;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.CloudConfig;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.helix.model.builder.HelixConfigScopeBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Path("/clusters")
+public class CloudAccessor extends AbstractHelixResource {
+  private static Logger _logger = LoggerFactory.getLogger(CloudAccessor.class.getName());
+
+  @PUT
+  @Path("{clusterId}/cloudconfig")
+  public Response addCloudConfig(@PathParam("clusterId") String clusterId, String content) {
+
+    HelixZkClient zkClient = getHelixZkClient();
+    if (!ZKUtil.isClusterSetup(clusterId, zkClient)) {
+      return notFound();
+    }
+
+    HelixAdmin admin = getHelixAdmin();
+    ZNRecord record;
+    try {
+      record = toZNRecord(content);
+    } catch (IOException e) {
+      _logger.error("Failed to deserialize user's input " + content + ", Exception: " + e);
+      return badRequest("Input is not a vaild ZNRecord!");
+    }
+
+    try {
+      CloudConfig cloudConfig = new CloudConfig.Builder(record).build();
+      admin.addCloudConfig(clusterId, cloudConfig);
+    } catch (Exception ex) {
+      _logger.error("Error in adding a CloudConfig to cluster: " + clusterId, ex);
+      return badRequest(ex.getMessage());
+    }
+
+    return OK();
+  }
+
+  @GET
+  @Path("{clusterId}/cloudconfig")
+  public Response getCloudConfig(@PathParam("clusterId") String clusterId) {
+
+    HelixZkClient zkClient = getHelixZkClient();
+    if (!ZKUtil.isClusterSetup(clusterId, zkClient)) {
+      return notFound();
+    }
+
+    ConfigAccessor configAccessor = new ConfigAccessor(zkClient);
+    CloudConfig cloudConfig = configAccessor.getCloudConfig(clusterId);
+
+    if (cloudConfig != null) {
+      return JSONRepresentation(cloudConfig.getRecord());
+    }
+
+    return notFound();
+  }
+
+  @POST
+  @Path("{clusterId}/cloudconfig")
+  public Response updateCloudConfig(@PathParam("clusterId") String clusterId,
+      @QueryParam("command") String commandStr, String content) {
+
+    HelixZkClient zkClient = getHelixZkClient();
+    if (!ZKUtil.isClusterSetup(clusterId, zkClient)) {
+      return notFound();
+    }
+
+    // Here to update cloud config
+    Command command;
+    if (commandStr == null || commandStr.isEmpty()) {
+      command = Command.update; // Default behavior
+    } else {
+      try {
+        command = getCommand(commandStr);
+      } catch (HelixException ex) {
+        return badRequest(ex.getMessage());
+      }
+    }
+
+    HelixAdmin admin = getHelixAdmin();
+    try {
+      switch (command) {
+        case delete: {
+          admin.removeCloudConfig(clusterId);
+        }
+        break;
+        case update: {
+          ZNRecord record;
 
 Review comment:
   Done.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] dasahcc commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig

Posted by GitBox <gi...@apache.org>.
dasahcc commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig
URL: https://github.com/apache/helix/pull/675#discussion_r363902937
 
 

 ##########
 File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/CloudAccessor.java
 ##########
 @@ -0,0 +1,152 @@
+package org.apache.helix.rest.server.resources.helix;
+
+/*
+ * 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.
+ */
+
+import java.io.IOException;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.Response;
+import org.apache.helix.ConfigAccessor;
+import org.apache.helix.HelixAdmin;
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.manager.zk.ZKUtil;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.CloudConfig;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.helix.model.builder.HelixConfigScopeBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Path("/clusters")
+public class CloudAccessor extends AbstractHelixResource {
+  private static Logger _logger = LoggerFactory.getLogger(CloudAccessor.class.getName());
+
+  @PUT
+  @Path("{clusterId}/cloudconfig")
+  public Response addCloudConfig(@PathParam("clusterId") String clusterId, String content) {
+
+    HelixZkClient zkClient = getHelixZkClient();
+    if (!ZKUtil.isClusterSetup(clusterId, zkClient)) {
+      return notFound();
+    }
+
+    HelixAdmin admin = getHelixAdmin();
+    ZNRecord record;
+    try {
+      record = toZNRecord(content);
+    } catch (IOException e) {
+      _logger.error("Failed to deserialize user's input " + content + ", Exception: " + e);
+      return badRequest("Input is not a vaild ZNRecord!");
+    }
+
+    try {
+      CloudConfig cloudConfig = new CloudConfig.Builder(record).build();
+      admin.addCloudConfig(clusterId, cloudConfig);
+    } catch (Exception ex) {
+      _logger.error("Error in adding a CloudConfig to cluster: " + clusterId, ex);
+      return badRequest(ex.getMessage());
+    }
+
+    return OK();
+  }
+
+  @GET
+  @Path("{clusterId}/cloudconfig")
+  public Response getCloudConfig(@PathParam("clusterId") String clusterId) {
+
+    HelixZkClient zkClient = getHelixZkClient();
+    if (!ZKUtil.isClusterSetup(clusterId, zkClient)) {
+      return notFound();
+    }
+
+    ConfigAccessor configAccessor = new ConfigAccessor(zkClient);
+    CloudConfig cloudConfig = configAccessor.getCloudConfig(clusterId);
+
+    if (cloudConfig != null) {
+      return JSONRepresentation(cloudConfig.getRecord());
+    }
+
+    return notFound();
+  }
+
+  @POST
+  @Path("{clusterId}/cloudconfig")
+  public Response updateCloudConfig(@PathParam("clusterId") String clusterId,
+      @QueryParam("command") String commandStr, String content) {
+
+    HelixZkClient zkClient = getHelixZkClient();
+    if (!ZKUtil.isClusterSetup(clusterId, zkClient)) {
+      return notFound();
+    }
+
+    // Here to update cloud config
+    Command command;
+    if (commandStr == null || commandStr.isEmpty()) {
+      command = Command.update; // Default behavior
+    } else {
+      try {
+        command = getCommand(commandStr);
+      } catch (HelixException ex) {
+        return badRequest(ex.getMessage());
+      }
+    }
+
+    HelixAdmin admin = getHelixAdmin();
+    try {
+      switch (command) {
+        case delete: {
+          admin.removeCloudConfig(clusterId);
+        }
+        break;
+        case update: {
+          ZNRecord record;
 
 Review comment:
   move record read before command switch.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] dasahcc commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig

Posted by GitBox <gi...@apache.org>.
dasahcc commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig
URL: https://github.com/apache/helix/pull/675#discussion_r363887244
 
 

 ##########
 File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/CloudAccessor.java
 ##########
 @@ -0,0 +1,152 @@
+package org.apache.helix.rest.server.resources.helix;
+
+/*
+ * 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.
+ */
+
+import java.io.IOException;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.Response;
+import org.apache.helix.ConfigAccessor;
+import org.apache.helix.HelixAdmin;
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.manager.zk.ZKUtil;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.CloudConfig;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.helix.model.builder.HelixConfigScopeBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Path("/clusters")
 
 Review comment:
   Does this work? It conflict with ClusterAccessor. It would be better to merge the logic into ClusterAccessor.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] dasahcc merged pull request #675: Add REST API for Cluster Creation with CloudConfig

Posted by GitBox <gi...@apache.org>.
dasahcc merged pull request #675: Add REST API for Cluster Creation with CloudConfig
URL: https://github.com/apache/helix/pull/675
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] alirezazamani commented on issue #675: Add REST API for Cluster Creation with CloudConfig

Posted by GitBox <gi...@apache.org>.
alirezazamani commented on issue #675: Add REST API for Cluster Creation with CloudConfig
URL: https://github.com/apache/helix/pull/675#issuecomment-576025796
 
 
   This PR is ready to be merged, approved by @dasahcc.
   
   Final Commit Message:
   Title:
   Add REST API for Cluster Creation with CloudConfig
   
   Body:
   This commit contains the relevant REST APIs for cluster creation.
   This change allows user to create cluster with CloudConfig.
   Multiple tests have been added in order to check the functionality of REST calls.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] dasahcc commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig

Posted by GitBox <gi...@apache.org>.
dasahcc commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig
URL: https://github.com/apache/helix/pull/675#discussion_r363901582
 
 

 ##########
 File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/CloudAccessor.java
 ##########
 @@ -0,0 +1,152 @@
+package org.apache.helix.rest.server.resources.helix;
+
+/*
+ * 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.
+ */
+
+import java.io.IOException;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.Response;
+import org.apache.helix.ConfigAccessor;
+import org.apache.helix.HelixAdmin;
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.manager.zk.ZKUtil;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.CloudConfig;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.helix.model.builder.HelixConfigScopeBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Path("/clusters")
+public class CloudAccessor extends AbstractHelixResource {
+  private static Logger _logger = LoggerFactory.getLogger(CloudAccessor.class.getName());
+
+  @PUT
+  @Path("{clusterId}/cloudconfig")
+  public Response addCloudConfig(@PathParam("clusterId") String clusterId, String content) {
+
+    HelixZkClient zkClient = getHelixZkClient();
+    if (!ZKUtil.isClusterSetup(clusterId, zkClient)) {
+      return notFound();
+    }
+
+    HelixAdmin admin = getHelixAdmin();
+    ZNRecord record;
+    try {
+      record = toZNRecord(content);
+    } catch (IOException e) {
+      _logger.error("Failed to deserialize user's input " + content + ", Exception: " + e);
+      return badRequest("Input is not a vaild ZNRecord!");
+    }
+
+    try {
+      CloudConfig cloudConfig = new CloudConfig.Builder(record).build();
+      admin.addCloudConfig(clusterId, cloudConfig);
+    } catch (Exception ex) {
 
 Review comment:
   Let's have another catch for HelixException to differentiate it with other general error. It could be validation exception.
   
   Same as following places.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] alirezazamani commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig

Posted by GitBox <gi...@apache.org>.
alirezazamani commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig
URL: https://github.com/apache/helix/pull/675#discussion_r366102366
 
 

 ##########
 File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/CloudAccessor.java
 ##########
 @@ -0,0 +1,152 @@
+package org.apache.helix.rest.server.resources.helix;
+
+/*
+ * 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.
+ */
+
+import java.io.IOException;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.Response;
+import org.apache.helix.ConfigAccessor;
+import org.apache.helix.HelixAdmin;
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.manager.zk.ZKUtil;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.CloudConfig;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.helix.model.builder.HelixConfigScopeBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Path("/clusters")
+public class CloudAccessor extends AbstractHelixResource {
+  private static Logger _logger = LoggerFactory.getLogger(CloudAccessor.class.getName());
+
+  @PUT
+  @Path("{clusterId}/cloudconfig")
+  public Response addCloudConfig(@PathParam("clusterId") String clusterId, String content) {
+
+    HelixZkClient zkClient = getHelixZkClient();
+    if (!ZKUtil.isClusterSetup(clusterId, zkClient)) {
 
 Review comment:
   Done.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] alirezazamani commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig

Posted by GitBox <gi...@apache.org>.
alirezazamani commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig
URL: https://github.com/apache/helix/pull/675#discussion_r366100681
 
 

 ##########
 File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ClusterAccessor.java
 ##########
 @@ -132,18 +133,42 @@ public Response getClusterInfo(@PathParam("clusterId") String clusterId) {
   @PUT
   @Path("{clusterId}")
   public Response createCluster(@PathParam("clusterId") String clusterId,
-      @DefaultValue("false") @QueryParam("recreate") String recreate) {
+      @DefaultValue("false") @QueryParam("recreate") String recreate,
+      @DefaultValue("false") @QueryParam("addCloudConfig") String addCloudConfig,
+      String content) {
+
     boolean recreateIfExists = Boolean.valueOf(recreate);
+    boolean cloudConfigIncluded = Boolean.valueOf(addCloudConfig);
+
+
     ClusterSetup clusterSetup = getClusterSetup();
 
-    try {
-      clusterSetup.addCluster(clusterId, recreateIfExists);
-    } catch (Exception ex) {
-      _logger.error("Failed to create cluster " + clusterId + ", exception: " + ex);
-      return serverError(ex);
-    }
+    if (!cloudConfigIncluded) {
 
 Review comment:
   Done.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] alirezazamani commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig

Posted by GitBox <gi...@apache.org>.
alirezazamani commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig
URL: https://github.com/apache/helix/pull/675#discussion_r366103680
 
 

 ##########
 File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/CloudAccessor.java
 ##########
 @@ -0,0 +1,152 @@
+package org.apache.helix.rest.server.resources.helix;
+
+/*
+ * 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.
+ */
+
+import java.io.IOException;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.Response;
+import org.apache.helix.ConfigAccessor;
+import org.apache.helix.HelixAdmin;
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.manager.zk.ZKUtil;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.CloudConfig;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.helix.model.builder.HelixConfigScopeBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Path("/clusters")
+public class CloudAccessor extends AbstractHelixResource {
+  private static Logger _logger = LoggerFactory.getLogger(CloudAccessor.class.getName());
+
+  @PUT
+  @Path("{clusterId}/cloudconfig")
+  public Response addCloudConfig(@PathParam("clusterId") String clusterId, String content) {
+
+    HelixZkClient zkClient = getHelixZkClient();
+    if (!ZKUtil.isClusterSetup(clusterId, zkClient)) {
+      return notFound();
+    }
+
+    HelixAdmin admin = getHelixAdmin();
+    ZNRecord record;
+    try {
+      record = toZNRecord(content);
+    } catch (IOException e) {
+      _logger.error("Failed to deserialize user's input " + content + ", Exception: " + e);
+      return badRequest("Input is not a vaild ZNRecord!");
+    }
+
+    try {
+      CloudConfig cloudConfig = new CloudConfig.Builder(record).build();
+      admin.addCloudConfig(clusterId, cloudConfig);
+    } catch (Exception ex) {
 
 Review comment:
   Fixed.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] dasahcc commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig

Posted by GitBox <gi...@apache.org>.
dasahcc commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig
URL: https://github.com/apache/helix/pull/675#discussion_r363902396
 
 

 ##########
 File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/CloudAccessor.java
 ##########
 @@ -0,0 +1,152 @@
+package org.apache.helix.rest.server.resources.helix;
+
+/*
+ * 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.
+ */
+
+import java.io.IOException;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.Response;
+import org.apache.helix.ConfigAccessor;
+import org.apache.helix.HelixAdmin;
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.manager.zk.ZKUtil;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.CloudConfig;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.helix.model.builder.HelixConfigScopeBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Path("/clusters")
+public class CloudAccessor extends AbstractHelixResource {
+  private static Logger _logger = LoggerFactory.getLogger(CloudAccessor.class.getName());
+
+  @PUT
+  @Path("{clusterId}/cloudconfig")
+  public Response addCloudConfig(@PathParam("clusterId") String clusterId, String content) {
+
+    HelixZkClient zkClient = getHelixZkClient();
+    if (!ZKUtil.isClusterSetup(clusterId, zkClient)) {
 
 Review comment:
   This is not a good practice here. User may get confused with 404 and without any other information here. Let's have clear message that cluster is not properly setup.
   
   Same as following places.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] alirezazamani commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig

Posted by GitBox <gi...@apache.org>.
alirezazamani commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig
URL: https://github.com/apache/helix/pull/675#discussion_r367211070
 
 

 ##########
 File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ClusterAccessor.java
 ##########
 @@ -514,6 +533,119 @@ private boolean doesClusterExist(String cluster) {
     return ZKUtil.isClusterSetup(cluster, zkClient);
   }
 
+  @PUT
+  @Path("{clusterId}/cloudconfig")
+  public Response addCloudConfig(@PathParam("clusterId") String clusterId, String content) {
+
+    HelixZkClient zkClient = getHelixZkClient();
+    if (!ZKUtil.isClusterSetup(clusterId, zkClient)) {
+      return notFound("Cluster is not properly setup!");
+    }
+
+    HelixAdmin admin = getHelixAdmin();
+    ZNRecord record;
+    try {
+      record = toZNRecord(content);
+    } catch (IOException e) {
+      _logger.error("Failed to deserialize user's input " + content + ", Exception: " + e);
+      return badRequest("Input is not a vaild ZNRecord!");
+    }
+
+    try {
+      CloudConfig cloudConfig = new CloudConfig.Builder(record).build();
+      admin.addCloudConfig(clusterId, cloudConfig);
+    } catch (HelixException ex) {
+      _logger.error("Error in adding a CloudConfig to cluster: " + clusterId, ex);
+      return badRequest(ex.getMessage());
+    } catch (Exception ex) {
+      _logger.error("Cannot add CloudConfig to cluster: " + clusterId, ex);
+      return badRequest(ex.getMessage());
 
 Review comment:
   Fixed. 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] alirezazamani commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig

Posted by GitBox <gi...@apache.org>.
alirezazamani commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig
URL: https://github.com/apache/helix/pull/675#discussion_r366616150
 
 

 ##########
 File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/CloudAccessor.java
 ##########
 @@ -0,0 +1,152 @@
+package org.apache.helix.rest.server.resources.helix;
+
+/*
+ * 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.
+ */
+
+import java.io.IOException;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.Response;
+import org.apache.helix.ConfigAccessor;
+import org.apache.helix.HelixAdmin;
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.manager.zk.ZKUtil;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.CloudConfig;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.helix.model.builder.HelixConfigScopeBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Path("/clusters")
+public class CloudAccessor extends AbstractHelixResource {
+  private static Logger _logger = LoggerFactory.getLogger(CloudAccessor.class.getName());
+
+  @PUT
+  @Path("{clusterId}/cloudconfig")
+  public Response addCloudConfig(@PathParam("clusterId") String clusterId, String content) {
+
+    HelixZkClient zkClient = getHelixZkClient();
+    if (!ZKUtil.isClusterSetup(clusterId, zkClient)) {
+      return notFound();
+    }
+
+    HelixAdmin admin = getHelixAdmin();
+    ZNRecord record;
+    try {
+      record = toZNRecord(content);
+    } catch (IOException e) {
+      _logger.error("Failed to deserialize user's input " + content + ", Exception: " + e);
+      return badRequest("Input is not a vaild ZNRecord!");
+    }
 
 Review comment:
   As we discussed offline, sine the output of toZNRecord function is a record, it is not easy to move try/catch to lower level. 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] dasahcc commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig

Posted by GitBox <gi...@apache.org>.
dasahcc commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig
URL: https://github.com/apache/helix/pull/675#discussion_r363896739
 
 

 ##########
 File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ClusterAccessor.java
 ##########
 @@ -132,18 +133,42 @@ public Response getClusterInfo(@PathParam("clusterId") String clusterId) {
   @PUT
   @Path("{clusterId}")
   public Response createCluster(@PathParam("clusterId") String clusterId,
-      @DefaultValue("false") @QueryParam("recreate") String recreate) {
+      @DefaultValue("false") @QueryParam("recreate") String recreate,
+      @DefaultValue("false") @QueryParam("addCloudConfig") String addCloudConfig,
+      String content) {
+
     boolean recreateIfExists = Boolean.valueOf(recreate);
+    boolean cloudConfigIncluded = Boolean.valueOf(addCloudConfig);
+
+
     ClusterSetup clusterSetup = getClusterSetup();
 
-    try {
-      clusterSetup.addCluster(clusterId, recreateIfExists);
-    } catch (Exception ex) {
-      _logger.error("Failed to create cluster " + clusterId + ", exception: " + ex);
-      return serverError(ex);
-    }
+    if (!cloudConfigIncluded) {
 
 Review comment:
   Let's reorganize the code. There are duplicated pieces.
   
   You can:
   
   1. Define CloudConfig object
   2. Create this object if enabled and move this piece of code to a separate function. Otherwise, keep it as null.
   3. Call the addCluster with CloudConfig object.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] dasahcc commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig

Posted by GitBox <gi...@apache.org>.
dasahcc commented on a change in pull request #675: Add REST API for Cluster Creation with CloudConfig
URL: https://github.com/apache/helix/pull/675#discussion_r367131650
 
 

 ##########
 File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ClusterAccessor.java
 ##########
 @@ -514,6 +533,119 @@ private boolean doesClusterExist(String cluster) {
     return ZKUtil.isClusterSetup(cluster, zkClient);
   }
 
+  @PUT
+  @Path("{clusterId}/cloudconfig")
+  public Response addCloudConfig(@PathParam("clusterId") String clusterId, String content) {
+
+    HelixZkClient zkClient = getHelixZkClient();
+    if (!ZKUtil.isClusterSetup(clusterId, zkClient)) {
+      return notFound("Cluster is not properly setup!");
+    }
+
+    HelixAdmin admin = getHelixAdmin();
+    ZNRecord record;
+    try {
+      record = toZNRecord(content);
+    } catch (IOException e) {
+      _logger.error("Failed to deserialize user's input " + content + ", Exception: " + e);
+      return badRequest("Input is not a vaild ZNRecord!");
+    }
+
+    try {
+      CloudConfig cloudConfig = new CloudConfig.Builder(record).build();
+      admin.addCloudConfig(clusterId, cloudConfig);
+    } catch (HelixException ex) {
+      _logger.error("Error in adding a CloudConfig to cluster: " + clusterId, ex);
+      return badRequest(ex.getMessage());
+    } catch (Exception ex) {
+      _logger.error("Cannot add CloudConfig to cluster: " + clusterId, ex);
+      return badRequest(ex.getMessage());
 
 Review comment:
   This is not badRequest. It may be something related to internal server error. 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org