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

 ##########
 File path: 
helix-rest/src/main/java/org/apache/helix/rest/metadatastore/accessor/ZkRoutingDataWriter.java
 ##########
 @@ -146,31 +152,15 @@ public synchronized boolean deleteShardingKey(String 
realm, String shardingKey)
       if (_zkClient.isClosed()) {
         throw new IllegalStateException("ZkClient is closed!");
       }
-      ZNRecord znRecord =
-          _zkClient.readData(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + 
"/" + realm, true);
-      if (znRecord == null || !znRecord
-          .getListField(MetadataStoreRoutingConstants.ZNRECORD_LIST_FIELD_KEY)
-          .contains(shardingKey)) {
-        // This realm does not exist or shardingKey doesn't exist. Return true!
-        return true;
-      }
-      
znRecord.getListField(MetadataStoreRoutingConstants.ZNRECORD_LIST_FIELD_KEY)
-          .remove(shardingKey);
-      // Overwrite this ZNRecord with the sharding key removed
-      try {
-        _zkClient
-            .writeData(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/" + 
realm, znRecord);
-      } catch (Exception e) {
-        LOG.error(
-            "Failed to write the data back in deleteShardingKey()! Namespace: 
{}, Realm: {}, ShardingKey: {}",
-            _namespace, realm, shardingKey, e);
-        return false;
-      }
-      return true;
+      return deleteZkShardingKey(realm, shardingKey);
     }
 
-    // TODO: Forward the request to leader
-    return true;
+    String urlSuffix =
+        
constructUrlSuffix(MetadataStoreRoutingConstants.MSDS_GET_ALL_REALMS_ENDPOINT, 
realm,
+            MetadataStoreRoutingConstants.MSDS_GET_ALL_SHARDING_KEYS_ENDPOINT, 
shardingKey);
+    return forwardRequestToLeader(urlSuffix,
+        MetadataStoreRoutingConstants.HttpRequestForwardingVerbs.DELETE, 
"deleteShardingKey",
 
 Review comment:
   I still see a string literal here?

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