pkuwm commented on a change in pull request #788: WIP: Implement request 
forwarding for ZkRoutingDataWriter
URL: https://github.com/apache/helix/pull/788#discussion_r382391180
 
 

 ##########
 File path: 
helix-rest/src/main/java/org/apache/helix/rest/server/resources/metadatastore/MetadataStoreDirectoryAccessor.java
 ##########
 @@ -153,9 +161,13 @@ public Response getShardingKeys(@QueryParam("realm") 
String realm) {
   public Response addShardingKey(@PathParam("realm") String realm,
       @PathParam("sharding-key") String shardingKey) {
     try {
-      _metadataStoreDirectory.addShardingKey(_namespace, realm, shardingKey);
+      if (!_metadataStoreDirectory.addShardingKey(_namespace, realm, 
shardingKey)) {
+        return serverError();
 
 Review comment:
   I understand the possible situations and there is no need to tell the client 
the real reason (and it is impossible here because the api just returns true or 
false). I meant, include a message suggesting what to do like “server error , 
please contact admin” is helpful than plain response. Imagine, what would you 
get if you do curl this endpoint but server error? 
   Anyway, I have an issue Created to improve the response. You don’t have to 
do it here. Focus on the main task of the pr instead.

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to