[GitHub] [helix] pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to helix-rest

2020-01-14 Thread GitBox
pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to 
helix-rest
URL: https://github.com/apache/helix/pull/671#discussion_r33823
 
 

 ##
 File path: 
helix-rest/src/main/java/org/apache/helix/rest/server/resources/zookeeper/ZooKeeperAccessor.java
 ##
 @@ -0,0 +1,174 @@
+package org.apache.helix.rest.server.resources.zookeeper;
+
+/*
+ * 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.UnsupportedEncodingException;
+import java.util.Base64;
+import java.util.List;
+import java.util.Map;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.Response;
+
+import com.google.common.base.Enums;
+import com.google.common.collect.ImmutableMap;
+import org.apache.helix.AccessOption;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.rest.common.ContextPropertyKeys;
+import org.apache.helix.rest.server.ServerContext;
+import org.apache.helix.rest.server.resources.AbstractResource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * ZooKeeperAccessor provides methods for accessing ZooKeeper resources 
(ZNodes).
+ * It provides basic ZooKeeper features supported by ZkClient.
+ */
+@Path("/zookeeper")
+public class ZooKeeperAccessor extends AbstractResource {
+  private static final Logger LOG = 
LoggerFactory.getLogger(ZooKeeperAccessor.class.getName());
+  private ZkBaseDataAccessor _zkBaseDataAccessor;
+
+  public enum ZooKeeperCommand {
+exists,
+getBinaryData,
+getStringData,
+getChildren
+  }
+
+  @GET
+  @Path("{path: .+}")
+  public Response get(@PathParam("path") String path, @QueryParam("command") 
String commandStr) {
+ZooKeeperCommand cmd = getZooKeeperCommandIfPresent(commandStr);
+if (cmd == null) {
+  return badRequest("Invalid ZooKeeper command: " + commandStr);
+}
+
+// Lazily initialize ZkBaseDataAccessor
+ServerContext _serverContext =
+(ServerContext) 
_application.getProperties().get(ContextPropertyKeys.SERVER_CONTEXT.name());
+_zkBaseDataAccessor = _serverContext.getByteArrayZkBaseDataAccessor();
+
+// Need to prepend a "/" since JAX-RS regex removes it
+path = "/" + path;
+
+// Check that the path supplied is valid
+if (!isPathValid(path)) {
+  String errMsg = "The given path is not a valid ZooKeeper path: " + path;
+  LOG.info(errMsg);
+  return badRequest(errMsg);
+}
+
+switch (cmd) {
+  case exists:
+return exists(path);
+  case getBinaryData:
+  case getStringData:
+return getData(path, cmd);
+  case getChildren:
+return getChildren(path);
+  default:
+LOG.error("Unsupported command: " + commandStr);
+return badRequest("Unsupported command: " + commandStr);
 
 Review comment:
   Nit: one less string 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:
us...@infra.apache.org


With regards,
Apache Git Services

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



[GitHub] [helix] pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to helix-rest

2020-01-14 Thread GitBox
pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to 
helix-rest
URL: https://github.com/apache/helix/pull/671#discussion_r366658812
 
 

 ##
 File path: 
helix-rest/src/main/java/org/apache/helix/rest/server/resources/zookeeper/ZooKeeperAccessor.java
 ##
 @@ -0,0 +1,183 @@
+package org.apache.helix.rest.server.resources.zookeeper;
+
+/*
+ * 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.UnsupportedEncodingException;
+import java.util.Base64;
+import java.util.List;
+import java.util.Map;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.Response;
+
+import com.google.common.base.Enums;
+import com.google.common.collect.ImmutableMap;
+import org.apache.helix.AccessOption;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.rest.common.ContextPropertyKeys;
+import org.apache.helix.rest.server.ServerContext;
+import org.apache.helix.rest.server.resources.AbstractResource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * ZooKeeperAccessor provides methods for accessing ZooKeeper resources 
(ZNodes).
+ * It provides basic ZooKeeper features supported by ZkClient.
+ */
+@Path("/zookeeper")
+public class ZooKeeperAccessor extends AbstractResource {
+  private static final Logger LOG = 
LoggerFactory.getLogger(ZooKeeperAccessor.class.getName());
+  private ZkBaseDataAccessor _zkBaseDataAccessor;
+
+  public enum ZooKeeperCommand {
+exists, getBinaryData, getStringData, getChildren
+  }
+
+  @GET
+  @Path("{path: .+}")
+  public Response get(@PathParam("path") String path, @QueryParam("command") 
String commandStr) {
+ZooKeeperCommand cmd = getZooKeeperCommandIfPresent(commandStr);
+if (cmd == null) {
+  return badRequest("Invalid ZooKeeper command: " + commandStr);
+}
+
+// Lazily initialize ZkBaseDataAccessor
+ServerContext _serverContext =
+(ServerContext) 
_application.getProperties().get(ContextPropertyKeys.SERVER_CONTEXT.name());
+_zkBaseDataAccessor = _serverContext.getByteArrayZkBaseDataAccessor();
+
+// Need to prepend a "/" since JAX-RS regex removes it
+path = "/" + path;
+switch (cmd) {
+  case exists:
+return exists(path);
+  case getBinaryData:
+  case getStringData:
+return getData(path, cmd);
+  case getChildren:
+return getChildren(path);
+  default:
+LOG.error("Unsupported command :" + commandStr);
+return badRequest("Unsupported command :" + commandStr);
+}
+  }
+
+  /**
+   * Checks if a ZNode exists in the given path.
+   * @param path
+   * @return true if a ZNode exists, false otherwise
+   */
+  private Response exists(String path) {
+if (!isPathValid(path)) {
+  String errMsg = "exists(): The given path is not a valid ZooKeeper path: 
" + path;
+  LOG.info(errMsg);
+  return badRequest(errMsg);
+}
+
+Map result = 
ImmutableMap.of(ZooKeeperCommand.exists.name(),
+_zkBaseDataAccessor.exists(path, AccessOption.PERSISTENT));
+return JSONRepresentation(result);
+  }
+
+  /**
+   * Reads the given path from ZooKeeper and returns the binary data for the 
ZNode.
+   * @param path
+   * @param command denotes whether return type should be binary or String
+   * @return binary data in the ZNode
+   */
+  private Response getData(String path, ZooKeeperCommand command) {
+if (!isPathValid(path)) {
+  String errMsg = "getData(): The given path is not a valid ZooKeeper 
path: " + path;
+  LOG.info(errMsg);
+  return badRequest(errMsg);
+}
+
+if (_zkBaseDataAccessor.exists(path, AccessOption.PERSISTENT)) {
+  byte[] bytes = _zkBaseDataAccessor.get(path, null, 
AccessOption.PERSISTENT);
+  String s = new String(bytes);
+
+  switch (command) {
+case getBinaryData:
+  Map binaryResult =
+  ImmutableMap.of(ZooKeeperCommand.getBinaryData.name(), bytes);
+  // Note: this serialization (using ObjectMapper) will convert this 
byte[] into
+  // a Base64 String! The REST client (user) must 

[GitHub] [helix] pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to helix-rest

2020-01-14 Thread GitBox
pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to 
helix-rest
URL: https://github.com/apache/helix/pull/671#discussion_r366625838
 
 

 ##
 File path: 
helix-rest/src/main/java/org/apache/helix/rest/server/resources/zookeeper/ZooKeeperAccessor.java
 ##
 @@ -0,0 +1,181 @@
+package org.apache.helix.rest.server.resources.zookeeper;
+
+/*
+ * 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.UnsupportedEncodingException;
+import java.util.Base64;
+import java.util.List;
+import java.util.Map;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.Response;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.helix.AccessOption;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.rest.common.ContextPropertyKeys;
+import org.apache.helix.rest.server.ServerContext;
+import org.apache.helix.rest.server.resources.AbstractResource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * ZooKeeperAccessor provides methods for accessing ZooKeeper resources 
(ZNodes).
+ * It provides basic ZooKeeper features supported by ZkClient.
+ */
+@Path("/zookeeper")
+public class ZooKeeperAccessor extends AbstractResource {
+  private static final Logger LOG = 
LoggerFactory.getLogger(ZooKeeperAccessor.class.getName());
+  private ZkBaseDataAccessor _zkBaseDataAccessor;
+
+  public enum ZooKeeperCommand {
+exists, getBinaryData, getStringData, getChildren
+  }
+
+  @GET
+  @Path("{path: .+}")
+  public Response get(@PathParam("path") String path, @QueryParam("command") 
String commandStr) {
+ZooKeeperCommand cmd;
+try {
+  cmd = ZooKeeperCommand.valueOf(commandStr);
+} catch (Exception e) {
+  return badRequest("Invalid ZooKeeper command: " + commandStr);
+}
+
+// Lazily initialize ZkBaseDataAccessor
+ServerContext _serverContext =
+(ServerContext) 
_application.getProperties().get(ContextPropertyKeys.SERVER_CONTEXT.name());
+_zkBaseDataAccessor = _serverContext.getByteArrayZkBaseDataAccessor();
+
+// Need to prepend a "/" since JAX-RS regex removes it
+path = "/" + path;
+switch (cmd) {
+  case exists:
+return exists(path);
+  case getBinaryData:
+return getData(path, cmd);
+  case getStringData:
+return getData(path, cmd);
+  case getChildren:
+return getChildren(path);
+  default:
+LOG.error("Unsupported command :" + commandStr);
+return badRequest("Unsupported command :" + commandStr);
+}
+  }
+
+  /**
+   * Checks if a ZNode exists in the given path.
+   * @param path
+   * @return true if a ZNode exists, false otherwise
+   */
+  private Response exists(String path) {
+if (!isPathValid(path)) {
+  String errMsg = "exists(): The given path {} is not a valid ZooKeeper 
path!" + path;
+  LOG.error(errMsg);
+  return badRequest(errMsg);
+}
+
+Map result = 
ImmutableMap.of(ZooKeeperCommand.exists.name(),
+_zkBaseDataAccessor.exists(path, AccessOption.PERSISTENT));
+return JSONRepresentation(result);
+  }
+
+  /**
+   * Reads the given path from ZooKeeper and returns the binary data for the 
ZNode.
+   * @param path
+   * @param command denotes whether return type should be binary or String
+   * @return binary data in the ZNode
+   */
+  private Response getData(String path, ZooKeeperCommand command) {
+if (!isPathValid(path)) {
+  String errMsg = "getData(): The given path {} is not a valid ZooKeeper 
path!" + path;
+  LOG.error(errMsg);
+  return badRequest(errMsg);
+}
+
+if (_zkBaseDataAccessor.exists(path, AccessOption.PERSISTENT)) {
+  byte[] bytes = _zkBaseDataAccessor.get(path, null, 
AccessOption.PERSISTENT);
+  String s = new String(bytes);
+
+  switch (command) {
+case getBinaryData:
+  Map binaryResult =
+  ImmutableMap.of(ZooKeeperCommand.getBinaryData.name(), bytes);
+  // Note: this serialization (using ObjectMapper) will convert this 
byte[] into
+  // a Base64 String! 

[GitHub] [helix] pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to helix-rest

2020-01-14 Thread GitBox
pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to 
helix-rest
URL: https://github.com/apache/helix/pull/671#discussion_r366625838
 
 

 ##
 File path: 
helix-rest/src/main/java/org/apache/helix/rest/server/resources/zookeeper/ZooKeeperAccessor.java
 ##
 @@ -0,0 +1,181 @@
+package org.apache.helix.rest.server.resources.zookeeper;
+
+/*
+ * 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.UnsupportedEncodingException;
+import java.util.Base64;
+import java.util.List;
+import java.util.Map;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.Response;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.helix.AccessOption;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.rest.common.ContextPropertyKeys;
+import org.apache.helix.rest.server.ServerContext;
+import org.apache.helix.rest.server.resources.AbstractResource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * ZooKeeperAccessor provides methods for accessing ZooKeeper resources 
(ZNodes).
+ * It provides basic ZooKeeper features supported by ZkClient.
+ */
+@Path("/zookeeper")
+public class ZooKeeperAccessor extends AbstractResource {
+  private static final Logger LOG = 
LoggerFactory.getLogger(ZooKeeperAccessor.class.getName());
+  private ZkBaseDataAccessor _zkBaseDataAccessor;
+
+  public enum ZooKeeperCommand {
+exists, getBinaryData, getStringData, getChildren
+  }
+
+  @GET
+  @Path("{path: .+}")
+  public Response get(@PathParam("path") String path, @QueryParam("command") 
String commandStr) {
+ZooKeeperCommand cmd;
+try {
+  cmd = ZooKeeperCommand.valueOf(commandStr);
+} catch (Exception e) {
+  return badRequest("Invalid ZooKeeper command: " + commandStr);
+}
+
+// Lazily initialize ZkBaseDataAccessor
+ServerContext _serverContext =
+(ServerContext) 
_application.getProperties().get(ContextPropertyKeys.SERVER_CONTEXT.name());
+_zkBaseDataAccessor = _serverContext.getByteArrayZkBaseDataAccessor();
+
+// Need to prepend a "/" since JAX-RS regex removes it
+path = "/" + path;
+switch (cmd) {
+  case exists:
+return exists(path);
+  case getBinaryData:
+return getData(path, cmd);
+  case getStringData:
+return getData(path, cmd);
+  case getChildren:
+return getChildren(path);
+  default:
+LOG.error("Unsupported command :" + commandStr);
+return badRequest("Unsupported command :" + commandStr);
+}
+  }
+
+  /**
+   * Checks if a ZNode exists in the given path.
+   * @param path
+   * @return true if a ZNode exists, false otherwise
+   */
+  private Response exists(String path) {
+if (!isPathValid(path)) {
+  String errMsg = "exists(): The given path {} is not a valid ZooKeeper 
path!" + path;
+  LOG.error(errMsg);
+  return badRequest(errMsg);
+}
+
+Map result = 
ImmutableMap.of(ZooKeeperCommand.exists.name(),
+_zkBaseDataAccessor.exists(path, AccessOption.PERSISTENT));
+return JSONRepresentation(result);
+  }
+
+  /**
+   * Reads the given path from ZooKeeper and returns the binary data for the 
ZNode.
+   * @param path
+   * @param command denotes whether return type should be binary or String
+   * @return binary data in the ZNode
+   */
+  private Response getData(String path, ZooKeeperCommand command) {
+if (!isPathValid(path)) {
+  String errMsg = "getData(): The given path {} is not a valid ZooKeeper 
path!" + path;
+  LOG.error(errMsg);
+  return badRequest(errMsg);
+}
+
+if (_zkBaseDataAccessor.exists(path, AccessOption.PERSISTENT)) {
+  byte[] bytes = _zkBaseDataAccessor.get(path, null, 
AccessOption.PERSISTENT);
+  String s = new String(bytes);
+
+  switch (command) {
+case getBinaryData:
+  Map binaryResult =
+  ImmutableMap.of(ZooKeeperCommand.getBinaryData.name(), bytes);
+  // Note: this serialization (using ObjectMapper) will convert this 
byte[] into
+  // a Base64 String! 

[GitHub] [helix] pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to helix-rest

2020-01-14 Thread GitBox
pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to 
helix-rest
URL: https://github.com/apache/helix/pull/671#discussion_r366618869
 
 

 ##
 File path: 
helix-rest/src/main/java/org/apache/helix/rest/server/resources/zookeeper/ZooKeeperAccessor.java
 ##
 @@ -0,0 +1,183 @@
+package org.apache.helix.rest.server.resources.zookeeper;
+
+/*
+ * 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.UnsupportedEncodingException;
+import java.util.Base64;
+import java.util.List;
+import java.util.Map;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.Response;
+
+import com.google.common.base.Enums;
+import com.google.common.collect.ImmutableMap;
+import org.apache.helix.AccessOption;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.rest.common.ContextPropertyKeys;
+import org.apache.helix.rest.server.ServerContext;
+import org.apache.helix.rest.server.resources.AbstractResource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * ZooKeeperAccessor provides methods for accessing ZooKeeper resources 
(ZNodes).
+ * It provides basic ZooKeeper features supported by ZkClient.
+ */
+@Path("/zookeeper")
+public class ZooKeeperAccessor extends AbstractResource {
+  private static final Logger LOG = 
LoggerFactory.getLogger(ZooKeeperAccessor.class.getName());
+  private ZkBaseDataAccessor _zkBaseDataAccessor;
+
+  public enum ZooKeeperCommand {
+exists, getBinaryData, getStringData, getChildren
+  }
+
+  @GET
+  @Path("{path: .+}")
+  public Response get(@PathParam("path") String path, @QueryParam("command") 
String commandStr) {
+ZooKeeperCommand cmd = getZooKeeperCommandIfPresent(commandStr);
+if (cmd == null) {
+  return badRequest("Invalid ZooKeeper command: " + commandStr);
+}
+
+// Lazily initialize ZkBaseDataAccessor
+ServerContext _serverContext =
+(ServerContext) 
_application.getProperties().get(ContextPropertyKeys.SERVER_CONTEXT.name());
+_zkBaseDataAccessor = _serverContext.getByteArrayZkBaseDataAccessor();
+
+// Need to prepend a "/" since JAX-RS regex removes it
+path = "/" + path;
+switch (cmd) {
+  case exists:
+return exists(path);
+  case getBinaryData:
+  case getStringData:
+return getData(path, cmd);
+  case getChildren:
+return getChildren(path);
+  default:
+LOG.error("Unsupported command :" + commandStr);
+return badRequest("Unsupported command :" + commandStr);
 
 Review comment:
   Nit, space after colon? Just for friendly user experience.


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:
us...@infra.apache.org


With regards,
Apache Git Services

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



[GitHub] [helix] pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to helix-rest

2020-01-14 Thread GitBox
pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to 
helix-rest
URL: https://github.com/apache/helix/pull/671#discussion_r366615810
 
 

 ##
 File path: 
helix-rest/src/main/java/org/apache/helix/rest/server/resources/zookeeper/ZooKeeperAccessor.java
 ##
 @@ -0,0 +1,181 @@
+package org.apache.helix.rest.server.resources.zookeeper;
+
+/*
+ * 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.UnsupportedEncodingException;
+import java.util.Base64;
+import java.util.List;
+import java.util.Map;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.Response;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.helix.AccessOption;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.rest.common.ContextPropertyKeys;
+import org.apache.helix.rest.server.ServerContext;
+import org.apache.helix.rest.server.resources.AbstractResource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * ZooKeeperAccessor provides methods for accessing ZooKeeper resources 
(ZNodes).
+ * It provides basic ZooKeeper features supported by ZkClient.
+ */
+@Path("/zookeeper")
+public class ZooKeeperAccessor extends AbstractResource {
+  private static final Logger LOG = 
LoggerFactory.getLogger(ZooKeeperAccessor.class.getName());
+  private ZkBaseDataAccessor _zkBaseDataAccessor;
+
+  public enum ZooKeeperCommand {
+exists, getBinaryData, getStringData, getChildren
 
 Review comment:
   I get your point and understand we should keep the consistence on the REST 
API. So to balance the code level convention and also API 
convention(`?command=getChildren`), I think we can also lookup the command by 
display name like this:
   ```
   public enum ZooKeeperCommand {
GET_CHILDREN("getChildren");
   
private String displayName;
private ZooKeeperCommand(String displayName) {
this.displayName = displayName;
}
public String getDisplayName() {
return displayName;
}
   
private static final Map namesMap = 
Maps.newHashMapWithExpectedSize(ZooKeeperCommand.values().length);
   
static {
for (ZooKeeperCommand command : ZooKeeperCommand.values()) {
namesMap.put(command.getDisplayName(), command);
}
}
public static ZooKeeperCommand lookupByName(String name) {
return namesMap.get(name);
}
   }
   
   So we can follow both conventions. What do you think?
   ```


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:
us...@infra.apache.org


With regards,
Apache Git Services

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



[GitHub] [helix] pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to helix-rest

2020-01-13 Thread GitBox
pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to 
helix-rest
URL: https://github.com/apache/helix/pull/671#discussion_r365972384
 
 

 ##
 File path: 
helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/PropertyStoreAccessor.java
 ##
 @@ -55,8 +56,8 @@
   public Response getPropertyByPath(@PathParam("clusterId") String clusterId,
   @PathParam("path") String path) {
 path = "/" + path;
-if (!isPathValid(path)) {
-  LOG.info("The propertyStore path {} is invalid for cluster {}", path, 
clusterId);
+if (!ZooKeeperAccessor.isPathValid(path)) {
+  LOG.error("The propertyStore path {} is invalid for cluster {}", path, 
clusterId);
 
 Review comment:
   Nit, I don't think this error message is appropriate. I remembered @lei-xia 
reviewed this in the previous ticket, this path invalidation is not an error in 
the server. So keeping info (or warn) may be good enough.


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:
us...@infra.apache.org


With regards,
Apache Git Services

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



[GitHub] [helix] pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to helix-rest

2020-01-13 Thread GitBox
pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to 
helix-rest
URL: https://github.com/apache/helix/pull/671#discussion_r365975327
 
 

 ##
 File path: 
helix-rest/src/main/java/org/apache/helix/rest/server/resources/zookeeper/ZooKeeperAccessor.java
 ##
 @@ -0,0 +1,181 @@
+package org.apache.helix.rest.server.resources.zookeeper;
+
+/*
+ * 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.UnsupportedEncodingException;
+import java.util.Base64;
+import java.util.List;
+import java.util.Map;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.Response;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.helix.AccessOption;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.rest.common.ContextPropertyKeys;
+import org.apache.helix.rest.server.ServerContext;
+import org.apache.helix.rest.server.resources.AbstractResource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * ZooKeeperAccessor provides methods for accessing ZooKeeper resources 
(ZNodes).
+ * It provides basic ZooKeeper features supported by ZkClient.
+ */
+@Path("/zookeeper")
+public class ZooKeeperAccessor extends AbstractResource {
+  private static final Logger LOG = 
LoggerFactory.getLogger(ZooKeeperAccessor.class.getName());
+  private ZkBaseDataAccessor _zkBaseDataAccessor;
+
+  public enum ZooKeeperCommand {
+exists, getBinaryData, getStringData, getChildren
+  }
+
+  @GET
+  @Path("{path: .+}")
+  public Response get(@PathParam("path") String path, @QueryParam("command") 
String commandStr) {
+ZooKeeperCommand cmd;
+try {
+  cmd = ZooKeeperCommand.valueOf(commandStr);
+} catch (Exception e) {
+  return badRequest("Invalid ZooKeeper command: " + commandStr);
+}
+
+// Lazily initialize ZkBaseDataAccessor
+ServerContext _serverContext =
+(ServerContext) 
_application.getProperties().get(ContextPropertyKeys.SERVER_CONTEXT.name());
+_zkBaseDataAccessor = _serverContext.getByteArrayZkBaseDataAccessor();
+
+// Need to prepend a "/" since JAX-RS regex removes it
+path = "/" + path;
+switch (cmd) {
+  case exists:
+return exists(path);
+  case getBinaryData:
+return getData(path, cmd);
+  case getStringData:
+return getData(path, cmd);
+  case getChildren:
+return getChildren(path);
+  default:
+LOG.error("Unsupported command :" + commandStr);
+return badRequest("Unsupported command :" + commandStr);
+}
+  }
+
+  /**
+   * Checks if a ZNode exists in the given path.
+   * @param path
+   * @return true if a ZNode exists, false otherwise
+   */
+  private Response exists(String path) {
+if (!isPathValid(path)) {
+  String errMsg = "exists(): The given path {} is not a valid ZooKeeper 
path!" + path;
+  LOG.error(errMsg);
+  return badRequest(errMsg);
+}
+
+Map result = 
ImmutableMap.of(ZooKeeperCommand.exists.name(),
+_zkBaseDataAccessor.exists(path, AccessOption.PERSISTENT));
+return JSONRepresentation(result);
+  }
+
+  /**
+   * Reads the given path from ZooKeeper and returns the binary data for the 
ZNode.
+   * @param path
+   * @param command denotes whether return type should be binary or String
+   * @return binary data in the ZNode
+   */
+  private Response getData(String path, ZooKeeperCommand command) {
+if (!isPathValid(path)) {
+  String errMsg = "getData(): The given path {} is not a valid ZooKeeper 
path!" + path;
+  LOG.error(errMsg);
+  return badRequest(errMsg);
+}
+
+if (_zkBaseDataAccessor.exists(path, AccessOption.PERSISTENT)) {
+  byte[] bytes = _zkBaseDataAccessor.get(path, null, 
AccessOption.PERSISTENT);
+  String s = new String(bytes);
+
+  switch (command) {
+case getBinaryData:
+  Map binaryResult =
+  ImmutableMap.of(ZooKeeperCommand.getBinaryData.name(), bytes);
+  // Note: this serialization (using ObjectMapper) will convert this 
byte[] into
+  // a Base64 String! 

[GitHub] [helix] pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to helix-rest

2020-01-13 Thread GitBox
pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to 
helix-rest
URL: https://github.com/apache/helix/pull/671#discussion_r365976198
 
 

 ##
 File path: 
helix-rest/src/main/java/org/apache/helix/rest/server/resources/zookeeper/ZooKeeperAccessor.java
 ##
 @@ -0,0 +1,181 @@
+package org.apache.helix.rest.server.resources.zookeeper;
+
+/*
+ * 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.UnsupportedEncodingException;
+import java.util.Base64;
+import java.util.List;
+import java.util.Map;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.Response;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.helix.AccessOption;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.rest.common.ContextPropertyKeys;
+import org.apache.helix.rest.server.ServerContext;
+import org.apache.helix.rest.server.resources.AbstractResource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * ZooKeeperAccessor provides methods for accessing ZooKeeper resources 
(ZNodes).
+ * It provides basic ZooKeeper features supported by ZkClient.
+ */
+@Path("/zookeeper")
+public class ZooKeeperAccessor extends AbstractResource {
+  private static final Logger LOG = 
LoggerFactory.getLogger(ZooKeeperAccessor.class.getName());
+  private ZkBaseDataAccessor _zkBaseDataAccessor;
+
+  public enum ZooKeeperCommand {
+exists, getBinaryData, getStringData, getChildren
+  }
+
+  @GET
+  @Path("{path: .+}")
+  public Response get(@PathParam("path") String path, @QueryParam("command") 
String commandStr) {
+ZooKeeperCommand cmd;
+try {
+  cmd = ZooKeeperCommand.valueOf(commandStr);
+} catch (Exception e) {
+  return badRequest("Invalid ZooKeeper command: " + commandStr);
+}
+
+// Lazily initialize ZkBaseDataAccessor
+ServerContext _serverContext =
+(ServerContext) 
_application.getProperties().get(ContextPropertyKeys.SERVER_CONTEXT.name());
+_zkBaseDataAccessor = _serverContext.getByteArrayZkBaseDataAccessor();
+
+// Need to prepend a "/" since JAX-RS regex removes it
+path = "/" + path;
+switch (cmd) {
+  case exists:
+return exists(path);
+  case getBinaryData:
+return getData(path, cmd);
+  case getStringData:
+return getData(path, cmd);
+  case getChildren:
+return getChildren(path);
+  default:
+LOG.error("Unsupported command :" + commandStr);
+return badRequest("Unsupported command :" + commandStr);
+}
+  }
+
+  /**
+   * Checks if a ZNode exists in the given path.
+   * @param path
+   * @return true if a ZNode exists, false otherwise
+   */
+  private Response exists(String path) {
+if (!isPathValid(path)) {
+  String errMsg = "exists(): The given path {} is not a valid ZooKeeper 
path!" + path;
+  LOG.error(errMsg);
+  return badRequest(errMsg);
+}
+
+Map result = 
ImmutableMap.of(ZooKeeperCommand.exists.name(),
+_zkBaseDataAccessor.exists(path, AccessOption.PERSISTENT));
+return JSONRepresentation(result);
+  }
+
+  /**
+   * Reads the given path from ZooKeeper and returns the binary data for the 
ZNode.
+   * @param path
+   * @param command denotes whether return type should be binary or String
+   * @return binary data in the ZNode
+   */
+  private Response getData(String path, ZooKeeperCommand command) {
+if (!isPathValid(path)) {
+  String errMsg = "getData(): The given path {} is not a valid ZooKeeper 
path!" + path;
+  LOG.error(errMsg);
+  return badRequest(errMsg);
+}
+
+if (_zkBaseDataAccessor.exists(path, AccessOption.PERSISTENT)) {
+  byte[] bytes = _zkBaseDataAccessor.get(path, null, 
AccessOption.PERSISTENT);
+  String s = new String(bytes);
+
+  switch (command) {
+case getBinaryData:
+  Map binaryResult =
+  ImmutableMap.of(ZooKeeperCommand.getBinaryData.name(), bytes);
+  // Note: this serialization (using ObjectMapper) will convert this 
byte[] into
+  // a Base64 String! 

[GitHub] [helix] pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to helix-rest

2020-01-13 Thread GitBox
pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to 
helix-rest
URL: https://github.com/apache/helix/pull/671#discussion_r365978278
 
 

 ##
 File path: 
helix-rest/src/main/java/org/apache/helix/rest/server/resources/zookeeper/ZooKeeperAccessor.java
 ##
 @@ -0,0 +1,181 @@
+package org.apache.helix.rest.server.resources.zookeeper;
+
+/*
+ * 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.UnsupportedEncodingException;
+import java.util.Base64;
+import java.util.List;
+import java.util.Map;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.Response;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.helix.AccessOption;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.rest.common.ContextPropertyKeys;
+import org.apache.helix.rest.server.ServerContext;
+import org.apache.helix.rest.server.resources.AbstractResource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * ZooKeeperAccessor provides methods for accessing ZooKeeper resources 
(ZNodes).
+ * It provides basic ZooKeeper features supported by ZkClient.
+ */
+@Path("/zookeeper")
+public class ZooKeeperAccessor extends AbstractResource {
+  private static final Logger LOG = 
LoggerFactory.getLogger(ZooKeeperAccessor.class.getName());
+  private ZkBaseDataAccessor _zkBaseDataAccessor;
+
+  public enum ZooKeeperCommand {
+exists, getBinaryData, getStringData, getChildren
+  }
+
+  @GET
+  @Path("{path: .+}")
+  public Response get(@PathParam("path") String path, @QueryParam("command") 
String commandStr) {
+ZooKeeperCommand cmd;
+try {
+  cmd = ZooKeeperCommand.valueOf(commandStr);
+} catch (Exception e) {
+  return badRequest("Invalid ZooKeeper command: " + commandStr);
+}
+
+// Lazily initialize ZkBaseDataAccessor
+ServerContext _serverContext =
+(ServerContext) 
_application.getProperties().get(ContextPropertyKeys.SERVER_CONTEXT.name());
+_zkBaseDataAccessor = _serverContext.getByteArrayZkBaseDataAccessor();
+
+// Need to prepend a "/" since JAX-RS regex removes it
+path = "/" + path;
+switch (cmd) {
+  case exists:
+return exists(path);
+  case getBinaryData:
+return getData(path, cmd);
+  case getStringData:
+return getData(path, cmd);
 
 Review comment:
   These 2 get data commands could be combined?
   ```
  case getBinaryData:
  case getStringData:
return getData(path, cmd);
   ```
   Or, can we split getData() to have 2 methods: getBinaryData() and 
getStringData(). I prefer this. Since we already check the command types here, 
it would be cleaner just to call the exact APIs, instead of checking the 
command in getData() again, which is passing the command enum and use an 
if-else to check.


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:
us...@infra.apache.org


With regards,
Apache Git Services

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



[GitHub] [helix] pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to helix-rest

2020-01-13 Thread GitBox
pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to 
helix-rest
URL: https://github.com/apache/helix/pull/671#discussion_r365968454
 
 

 ##
 File path: 
helix-rest/src/main/java/org/apache/helix/rest/server/resources/zookeeper/ZooKeeperAccessor.java
 ##
 @@ -0,0 +1,181 @@
+package org.apache.helix.rest.server.resources.zookeeper;
+
+/*
+ * 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.UnsupportedEncodingException;
+import java.util.Base64;
+import java.util.List;
+import java.util.Map;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.Response;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.helix.AccessOption;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.rest.common.ContextPropertyKeys;
+import org.apache.helix.rest.server.ServerContext;
+import org.apache.helix.rest.server.resources.AbstractResource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * ZooKeeperAccessor provides methods for accessing ZooKeeper resources 
(ZNodes).
+ * It provides basic ZooKeeper features supported by ZkClient.
+ */
+@Path("/zookeeper")
+public class ZooKeeperAccessor extends AbstractResource {
+  private static final Logger LOG = 
LoggerFactory.getLogger(ZooKeeperAccessor.class.getName());
+  private ZkBaseDataAccessor _zkBaseDataAccessor;
+
+  public enum ZooKeeperCommand {
+exists, getBinaryData, getStringData, getChildren
+  }
+
+  @GET
+  @Path("{path: .+}")
+  public Response get(@PathParam("path") String path, @QueryParam("command") 
String commandStr) {
+ZooKeeperCommand cmd;
+try {
+  cmd = ZooKeeperCommand.valueOf(commandStr);
+} catch (Exception e) {
+  return badRequest("Invalid ZooKeeper command: " + commandStr);
 
 Review comment:
   In my opinion, exceptions should not be used for control flow and could have 
performance implications. I believe there are a few better options to reach 
what you want :
   
   1. Guava's Enums: `Enums.getIfPresent()`.
   ```
   public static ZooKeeperCommand getIfPresent(String command) {
   return Enums.getIfPresent(ZooKeeperCommand.class, command).orNull();
   }
   ```
   2. Static map index: 
   ```
   private static final Map namesMap =
   Maps.newHashMapWithExpectedSize(ZooKeeperCommand.values().length);
   
   static {
   for (ZooKeeperCommand command : ZooKeeperCommand.values()) {
   namesMap.put(suit.name(), command);
   }
   }
   public static ZooKeeperCommand lookupByName(String name) {
   return namesMap.get(name);
   }
   ```


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:
us...@infra.apache.org


With regards,
Apache Git Services

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



[GitHub] [helix] pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to helix-rest

2020-01-13 Thread GitBox
pkuwm commented on a change in pull request #671: Add ZooKeeperAccessor to 
helix-rest
URL: https://github.com/apache/helix/pull/671#discussion_r365971210
 
 

 ##
 File path: 
helix-rest/src/main/java/org/apache/helix/rest/server/resources/zookeeper/ZooKeeperAccessor.java
 ##
 @@ -0,0 +1,181 @@
+package org.apache.helix.rest.server.resources.zookeeper;
+
+/*
+ * 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.UnsupportedEncodingException;
+import java.util.Base64;
+import java.util.List;
+import java.util.Map;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.Response;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.helix.AccessOption;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.rest.common.ContextPropertyKeys;
+import org.apache.helix.rest.server.ServerContext;
+import org.apache.helix.rest.server.resources.AbstractResource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * ZooKeeperAccessor provides methods for accessing ZooKeeper resources 
(ZNodes).
+ * It provides basic ZooKeeper features supported by ZkClient.
+ */
+@Path("/zookeeper")
+public class ZooKeeperAccessor extends AbstractResource {
+  private static final Logger LOG = 
LoggerFactory.getLogger(ZooKeeperAccessor.class.getName());
+  private ZkBaseDataAccessor _zkBaseDataAccessor;
+
+  public enum ZooKeeperCommand {
+exists, getBinaryData, getStringData, getChildren
 
 Review comment:
   Basically code style. From [oracle 
java](https://docs.oracle.com/javase/tutorial/java/javaOO/enum.html): An enum 
type is a special data type that enables for a variable to be a set of 
predefined **constants**. I would treat these enums as constants and make them 
follow constant's style. (Maybe you have your own reason: make them like what 
the legacy code looks.)


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:
us...@infra.apache.org


With regards,
Apache Git Services

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