[GitHub] [helix] pkuwm commented on a change in pull request #761: Add REST read endpoints to helix-rest for metadata store directory
pkuwm commented on a change in pull request #761: Add REST read endpoints to
helix-rest for metadata store directory
URL: https://github.com/apache/helix/pull/761#discussion_r382763795
##
File path:
helix-rest/src/main/java/org/apache/helix/rest/server/resources/metadatastore/MetadataStoreDirectoryAccessor.java
##
@@ -111,41 +135,74 @@ public Response
deleteMetadataStoreRealm(@PathParam("realm") String realm) {
}
/**
- * Gets sharding keys mapped at path "HTTP GET /sharding-keys" which returns
all sharding keys in
- * a namespace, or path "HTTP GET /sharding-keys?realm={realmName}" which
returns sharding keys in
- * a realm.
+ * Gets all sharding keys mapped at paths:
+ * - "HTTP GET /sharding-keys" which returns all sharding keys in a
namespace.
+ * - "HTTP GET /sharding-keys?realm={realm}" which returns sharding keys in
the realm.
+ * - "HTTP GET /sharding-keys?prefix={prefix}" which returns sharding keys
that have the prefix.
+ * -- JSON response example for this path:
+ * {
+ * "prefix": "/sharding/key",
+ * "shardingKeys": [{
+ * "realm": "testRealm2",
+ * "shardingKey": "/sharding/key/1/f"
+ *}, {
+ * "realm": "testRealm2",
+ * "shardingKey": "/sharding/key/1/e"
+ * }, {
+ * "realm": "testRealm1",
+ * "shardingKey": "/sharding/key/1/b"
+ * }, {
+ * "realm": "testRealm1",
+ * "shardingKey": "/sharding/key/1/a"
+ * }]
+ * }
*
- * @param realm Query param in endpoint path
- * @return Json representation of a map: shardingKeys -> collection of
sharding keys.
+ * - "HTTP GET /sharding-keys?realm={realm}&prefix={prefix}" which returns
sharding keys in the
+ * realm and that have the prefix.
+ *
+ * @param realm Query param in endpoint path: metadata store realm.
+ * @param prefix Query param in endpoint path: prefix substring of sharding
key.
+ * @return Json representation for the sharding keys.
*/
@GET
@Path("/sharding-keys")
- public Response getShardingKeys(@QueryParam("realm") String realm) {
-Map responseMap;
-Collection shardingKeys;
+ public Response getShardingKeys(@QueryParam("realm") String realm,
+ @QueryParam("prefix") String prefix) {
try {
- // If realm is not set in query param, the endpoint is: "/sharding-keys"
- // to get all sharding keys in a namespace.
- if (realm == null) {
-shardingKeys = _metadataStoreDirectory.getAllShardingKeys(_namespace);
-// To avoid allocating unnecessary resource, limit the map's capacity
only for
-// SHARDING_KEYS.
-responseMap = new HashMap<>(1);
+ if (realm == null && prefix == null) {
+// For endpoint: "/sharding-keys" to get all sharding keys in a
namespace.
+return getAllShardingKeys();
+ } else if (prefix == null) {
+// For endpoint: "/sharding-keys?realm={realm}"
+return getAllShardingKeysInRealm(realm);
+ } else if (realm == null) {
+// For endpoint: "/sharding-keys?prefix={prefix}"
+return getAllShardingKeysUnderPath(prefix);
} else {
-// For endpoint: "/sharding-keys?realm={realmName}"
-shardingKeys =
_metadataStoreDirectory.getAllShardingKeysInRealm(_namespace, realm);
-// To avoid allocating unnecessary resource, limit the map's capacity
only for
-// SHARDING_KEYS and SINGLE_METADATA_STORE_REALM.
-responseMap = new HashMap<>(2);
-
responseMap.put(MetadataStoreRoutingConstants.SINGLE_METADATA_STORE_REALM,
realm);
+// For endpoint: "/sharding-keys?realm={realm}&prefix={prefix}"
+return getRealmShardingKeysUnderPath(realm, prefix);
}
} catch (NoSuchElementException ex) {
return notFound(ex.getMessage());
}
+ }
-responseMap.put(MetadataStoreRoutingConstants.SHARDING_KEYS, shardingKeys);
-
-return JSONRepresentation(responseMap);
+ /**
+ * Gets all path-based sharding keys for a queried realm at endpoint:
+ * "GET /metadata-store-realms/{realm}/sharding-keys". This endpoint is
equivalent to
+ * the endpoint: "GET /sharding-keys?realm={realm}".
+ *
+ * @param realm Queried metadata store realm to get sharding keys.
+ * @return All path-based sharding keys in the queried realm.
+ */
+ @GET
+ @Path("/metadata-store-realms/{realm}/sharding-keys")
+ public Response getRealmShardingKeys(@PathParam("realm") String realm) {
+try {
+ return getAllShardingKeysInRealm(realm);
+} catch (NoSuchElementException ex) {
+ return notFound(ex.getMessage());
Review comment:
In your example, the **endpoint** is
`/metadata-store-realms/{realm}/sharding-keys` is already correct. But the
resource `test-realm` does not exist, and 404 is a **correct** response code.
If endpoint is incorrect, you would not get 404 but 400 (bad URI) instead.
So if you get 404, the first reaction is def
[GitHub] [helix] pkuwm commented on a change in pull request #761: Add REST read endpoints to helix-rest for metadata store directory
pkuwm commented on a change in pull request #761: Add REST read endpoints to
helix-rest for metadata store directory
URL: https://github.com/apache/helix/pull/761#discussion_r381862514
##
File path:
helix-rest/src/main/java/org/apache/helix/rest/server/resources/metadatastore/MetadataStoreDirectoryAccessor.java
##
@@ -111,41 +132,56 @@ public Response
deleteMetadataStoreRealm(@PathParam("realm") String realm) {
}
/**
- * Gets sharding keys mapped at path "HTTP GET /sharding-keys" which returns
all sharding keys in
- * a namespace, or path "HTTP GET /sharding-keys?realm={realmName}" which
returns sharding keys in
- * a realm.
+ * Gets all sharding keys mapped at paths:
+ * - "HTTP GET /sharding-keys" which returns all sharding keys in a
namespace.
+ * - "HTTP GET /sharding-keys?realm={realm}" which returns sharding keys in
the realm.
+ * - "HTTP GET /sharding-keys?prefix={prefix}" which returns sharding keys
that have the prefix.
+ * -- JSON response example for this path:
+ * {
+ * "prefix": "/sharding/key",
+ * "shardingKeys": [{
+ * "realm": "testRealm2",
+ * "shardingKey": "/sharding/key/1/f"
+ *}, {
+ * "realm": "testRealm2",
+ * "shardingKey": "/sharding/key/1/e"
+ * }, {
+ * "realm": "testRealm1",
+ * "shardingKey": "/sharding/key/1/b"
+ * }, {
+ * "realm": "testRealm1",
+ * "shardingKey": "/sharding/key/1/a"
+ * }]
+ * }
+ *
+ * - "HTTP GET /sharding-keys?realm={realm}&prefix={prefix}" which returns
sharding keys in the
+ * realm and that have the prefix.
*
- * @param realm Query param in endpoint path
- * @return Json representation of a map: shardingKeys -> collection of
sharding keys.
+ * @param realm Query param in endpoint path: metadata store realm.
+ * @param prefix Query param in endpoint path: prefix substring of sharding
key.
+ * @return Json representation for the sharding keys.
*/
@GET
@Path("/sharding-keys")
- public Response getShardingKeys(@QueryParam("realm") String realm) {
-Map responseMap;
-Collection shardingKeys;
+ public Response getShardingKeys(@QueryParam("realm") String realm,
+ @QueryParam("prefix") String prefix) {
try {
- // If realm is not set in query param, the endpoint is: "/sharding-keys"
- // to get all sharding keys in a namespace.
- if (realm == null) {
-shardingKeys = _metadataStoreDirectory.getAllShardingKeys(_namespace);
-// To avoid allocating unnecessary resource, limit the map's capacity
only for
-// SHARDING_KEYS.
-responseMap = new HashMap<>(1);
+ if (realm == null && prefix == null) {
+// For endpoint: "/sharding-keys" to get all sharding keys in a
namespace.
+return getAllShardingKeys();
+ } else if (prefix == null) {
+// For endpoint: "/sharding-keys?realm={realm}"
+return getAllShardingKeysInRealm(realm);
+ } else if (realm == null) {
+// For endpoint: "/sharding-keys?prefix={prefix}"
+return getAllShardingKeysUnderPath(prefix);
} else {
-// For endpoint: "/sharding-keys?realm={realmName}"
-shardingKeys =
_metadataStoreDirectory.getAllShardingKeysInRealm(_namespace, realm);
-// To avoid allocating unnecessary resource, limit the map's capacity
only for
-// SHARDING_KEYS and SINGLE_METADATA_STORE_REALM.
-responseMap = new HashMap<>(2);
-
responseMap.put(MetadataStoreRoutingConstants.SINGLE_METADATA_STORE_REALM,
realm);
+// For endpoint: "/sharding-keys?realm={realm}&prefix={prefix}"
+return getRealmShardingKeysUnderPath(realm, prefix);
Review comment:
I understand the specific case “/“ in getAllShardingKeysUnderPath could
return the same result of getAllShardingKeys. But what if
getAllShardingKeysUnderPath changes and returns different result?
- The Json responses are different for each case here. If you assign "/" to
a method, the response could have { "prefix": "/" }
- I would like to decouple two cases that having prefix
(getAllShardingKeysUnderPath ) and no prefix.
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 #761: Add REST read endpoints to helix-rest for metadata store directory
pkuwm commented on a change in pull request #761: Add REST read endpoints to
helix-rest for metadata store directory
URL: https://github.com/apache/helix/pull/761#discussion_r381053697
##
File path:
helix-rest/src/main/java/org/apache/helix/rest/server/resources/metadatastore/MetadataStoreDirectoryAccessor.java
##
@@ -111,41 +132,56 @@ public Response
deleteMetadataStoreRealm(@PathParam("realm") String realm) {
}
/**
- * Gets sharding keys mapped at path "HTTP GET /sharding-keys" which returns
all sharding keys in
- * a namespace, or path "HTTP GET /sharding-keys?realm={realmName}" which
returns sharding keys in
- * a realm.
+ * Gets all sharding keys mapped at paths:
+ * - "HTTP GET /sharding-keys" which returns all sharding keys in a
namespace.
+ * - "HTTP GET /sharding-keys?realm={realm}" which returns sharding keys in
the realm.
+ * - "HTTP GET /sharding-keys?prefix={prefix}" which returns sharding keys
that have the prefix.
+ * -- JSON response example for this path:
+ * {
+ * "prefix": "/sharding/key",
+ * "shardingKeys": [{
+ * "realm": "testRealm2",
+ * "shardingKey": "/sharding/key/1/f"
+ *}, {
+ * "realm": "testRealm2",
+ * "shardingKey": "/sharding/key/1/e"
+ * }, {
+ * "realm": "testRealm1",
+ * "shardingKey": "/sharding/key/1/b"
+ * }, {
+ * "realm": "testRealm1",
+ * "shardingKey": "/sharding/key/1/a"
+ * }]
+ * }
+ *
+ * - "HTTP GET /sharding-keys?realm={realm}&prefix={prefix}" which returns
sharding keys in the
+ * realm and that have the prefix.
*
- * @param realm Query param in endpoint path
- * @return Json representation of a map: shardingKeys -> collection of
sharding keys.
+ * @param realm Query param in endpoint path: metadata store realm.
+ * @param prefix Query param in endpoint path: prefix substring of sharding
key.
+ * @return Json representation for the sharding keys.
*/
@GET
@Path("/sharding-keys")
- public Response getShardingKeys(@QueryParam("realm") String realm) {
-Map responseMap;
-Collection shardingKeys;
+ public Response getShardingKeys(@QueryParam("realm") String realm,
+ @QueryParam("prefix") String prefix) {
try {
- // If realm is not set in query param, the endpoint is: "/sharding-keys"
- // to get all sharding keys in a namespace.
- if (realm == null) {
-shardingKeys = _metadataStoreDirectory.getAllShardingKeys(_namespace);
-// To avoid allocating unnecessary resource, limit the map's capacity
only for
-// SHARDING_KEYS.
-responseMap = new HashMap<>(1);
+ if (realm == null && prefix == null) {
+// For endpoint: "/sharding-keys" to get all sharding keys in a
namespace.
+return getAllShardingKeys();
+ } else if (prefix == null) {
+// For endpoint: "/sharding-keys?realm={realm}"
+return getAllShardingKeysInRealm(realm);
+ } else if (realm == null) {
+// For endpoint: "/sharding-keys?prefix={prefix}"
+return getAllShardingKeysUnderPath(prefix);
} else {
-// For endpoint: "/sharding-keys?realm={realmName}"
-shardingKeys =
_metadataStoreDirectory.getAllShardingKeysInRealm(_namespace, realm);
-// To avoid allocating unnecessary resource, limit the map's capacity
only for
-// SHARDING_KEYS and SINGLE_METADATA_STORE_REALM.
-responseMap = new HashMap<>(2);
-
responseMap.put(MetadataStoreRoutingConstants.SINGLE_METADATA_STORE_REALM,
realm);
+// For endpoint: "/sharding-keys?realm={realm}&prefix={prefix}"
+return getRealmShardingKeysUnderPath(realm, prefix);
Review comment:
> There is a good way to simplify the logic here utilizing the default
values as @dasahcc has mentioned. If the prefix is not provided, consider
assigning "/" to the prefix variable and use the same logic as the situation
where prefix exists.
@NealSun96 I don't think adding "/" to the prefix var is a better idea
because:
- The Json responses are different for each case here. If you assign "/" to
a method, the response could have `{ "prefix": "/" }`
- I've tried to find a good default value for this prefix. If I assign "/"
as a default value, I've no idea how to differentiate if prefix is not provide
or it is "/".
- The code logic to generate response map is different.
If I don't understand your idea, feel free to clarify it. Thanks for the tip.
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
---
[GitHub] [helix] pkuwm commented on a change in pull request #761: Add REST read endpoints to helix-rest for metadata store directory
pkuwm commented on a change in pull request #761: Add REST read endpoints to
helix-rest for metadata store directory
URL: https://github.com/apache/helix/pull/761#discussion_r381044344
##
File path:
helix-rest/src/test/java/org/apache/helix/rest/server/resources/zookeeper/TestMetadataStoreDirectoryAccessor.java
##
@@ -110,7 +110,33 @@ public void beforeClass() throws Exception {
_metadataStoreDirectory = new ZkMetadataStoreDirectory(routingZkAddrMap);
}
+ /*
+ * Tests REST endpoint: "GET
/namespaces/{namespace}/metadata-store-namespaces"
+ */
@Test
+ public void testGetAllNamespaces() throws IOException {
+String responseBody = get(TEST_NAMESPACE_URI_PREFIX +
"/metadata-store-namespaces", null,
+Response.Status.OK.getStatusCode(), true);
+
+// It is safe to cast the object and suppress warnings.
+@SuppressWarnings("unchecked")
+Map> queriedNamespacesMap =
+OBJECT_MAPPER.readValue(responseBody, Map.class);
+
+Assert.assertTrue(
+
queriedNamespacesMap.containsKey(MetadataStoreRoutingConstants.METADATA_STORE_NAMESPACES));
Review comment:
Sure. I was checking the response has this field. It is definitely more
accurate to check that the response has only expected fields.
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 #761: Add REST read endpoints to helix-rest for metadata store directory
pkuwm commented on a change in pull request #761: Add REST read endpoints to
helix-rest for metadata store directory
URL: https://github.com/apache/helix/pull/761#discussion_r381000691
##
File path:
helix-rest/src/main/java/org/apache/helix/rest/server/resources/metadatastore/MetadataStoreDirectoryAccessor.java
##
@@ -111,41 +135,74 @@ public Response
deleteMetadataStoreRealm(@PathParam("realm") String realm) {
}
/**
- * Gets sharding keys mapped at path "HTTP GET /sharding-keys" which returns
all sharding keys in
- * a namespace, or path "HTTP GET /sharding-keys?realm={realmName}" which
returns sharding keys in
- * a realm.
+ * Gets all sharding keys mapped at paths:
+ * - "HTTP GET /sharding-keys" which returns all sharding keys in a
namespace.
+ * - "HTTP GET /sharding-keys?realm={realm}" which returns sharding keys in
the realm.
+ * - "HTTP GET /sharding-keys?prefix={prefix}" which returns sharding keys
that have the prefix.
+ * -- JSON response example for this path:
+ * {
+ * "prefix": "/sharding/key",
+ * "shardingKeys": [{
+ * "realm": "testRealm2",
+ * "shardingKey": "/sharding/key/1/f"
+ *}, {
+ * "realm": "testRealm2",
+ * "shardingKey": "/sharding/key/1/e"
+ * }, {
+ * "realm": "testRealm1",
+ * "shardingKey": "/sharding/key/1/b"
+ * }, {
+ * "realm": "testRealm1",
+ * "shardingKey": "/sharding/key/1/a"
+ * }]
+ * }
*
- * @param realm Query param in endpoint path
- * @return Json representation of a map: shardingKeys -> collection of
sharding keys.
+ * - "HTTP GET /sharding-keys?realm={realm}&prefix={prefix}" which returns
sharding keys in the
+ * realm and that have the prefix.
+ *
+ * @param realm Query param in endpoint path: metadata store realm.
+ * @param prefix Query param in endpoint path: prefix substring of sharding
key.
+ * @return Json representation for the sharding keys.
*/
@GET
@Path("/sharding-keys")
- public Response getShardingKeys(@QueryParam("realm") String realm) {
-Map responseMap;
-Collection shardingKeys;
+ public Response getShardingKeys(@QueryParam("realm") String realm,
+ @QueryParam("prefix") String prefix) {
try {
- // If realm is not set in query param, the endpoint is: "/sharding-keys"
- // to get all sharding keys in a namespace.
- if (realm == null) {
-shardingKeys = _metadataStoreDirectory.getAllShardingKeys(_namespace);
-// To avoid allocating unnecessary resource, limit the map's capacity
only for
-// SHARDING_KEYS.
-responseMap = new HashMap<>(1);
+ if (realm == null && prefix == null) {
+// For endpoint: "/sharding-keys" to get all sharding keys in a
namespace.
+return getAllShardingKeys();
+ } else if (prefix == null) {
+// For endpoint: "/sharding-keys?realm={realm}"
+return getAllShardingKeysInRealm(realm);
+ } else if (realm == null) {
+// For endpoint: "/sharding-keys?prefix={prefix}"
+return getAllShardingKeysUnderPath(prefix);
} else {
-// For endpoint: "/sharding-keys?realm={realmName}"
-shardingKeys =
_metadataStoreDirectory.getAllShardingKeysInRealm(_namespace, realm);
-// To avoid allocating unnecessary resource, limit the map's capacity
only for
-// SHARDING_KEYS and SINGLE_METADATA_STORE_REALM.
-responseMap = new HashMap<>(2);
-
responseMap.put(MetadataStoreRoutingConstants.SINGLE_METADATA_STORE_REALM,
realm);
+// For endpoint: "/sharding-keys?realm={realm}&prefix={prefix}"
+return getRealmShardingKeysUnderPath(realm, prefix);
}
} catch (NoSuchElementException ex) {
return notFound(ex.getMessage());
}
+ }
-responseMap.put(MetadataStoreRoutingConstants.SHARDING_KEYS, shardingKeys);
-
-return JSONRepresentation(responseMap);
+ /**
+ * Gets all path-based sharding keys for a queried realm at endpoint:
+ * "GET /metadata-store-realms/{realm}/sharding-keys". This endpoint is
equivalent to
+ * the endpoint: "GET /sharding-keys?realm={realm}".
+ *
+ * @param realm Queried metadata store realm to get sharding keys.
+ * @return All path-based sharding keys in the queried realm.
+ */
+ @GET
+ @Path("/metadata-store-realms/{realm}/sharding-keys")
+ public Response getRealmShardingKeys(@PathParam("realm") String realm) {
+try {
+ return getAllShardingKeysInRealm(realm);
+} catch (NoSuchElementException ex) {
+ return notFound(ex.getMessage());
Review comment:
@NealSun96 Thanks for reviewing.
As you referred in the link, `400 Bad Request`: The request could not be
understood by the server due to **malformed syntax**. If you request a POST/PUT
with json payload which has syntax error, I believe `Bad Request` is more
suitable.
`404 Not Found` means the endpoint i
[GitHub] [helix] pkuwm commented on a change in pull request #761: Add REST read endpoints to helix-rest for metadata store directory
pkuwm commented on a change in pull request #761: Add REST read endpoints to
helix-rest for metadata store directory
URL: https://github.com/apache/helix/pull/761#discussion_r379284824
##
File path:
helix-rest/src/main/java/org/apache/helix/rest/server/resources/metadatastore/MetadataStoreDirectoryAccessor.java
##
@@ -111,41 +132,56 @@ public Response
deleteMetadataStoreRealm(@PathParam("realm") String realm) {
}
/**
- * Gets sharding keys mapped at path "HTTP GET /sharding-keys" which returns
all sharding keys in
- * a namespace, or path "HTTP GET /sharding-keys?realm={realmName}" which
returns sharding keys in
- * a realm.
+ * Gets all sharding keys mapped at paths:
+ * - "HTTP GET /sharding-keys" which returns all sharding keys in a
namespace.
+ * - "HTTP GET /sharding-keys?realm={realm}" which returns sharding keys in
the realm.
+ * - "HTTP GET /sharding-keys?prefix={prefix}" which returns sharding keys
that have the prefix.
+ * -- JSON response example for this path:
+ * {
+ * "prefix": "/sharding/key",
+ * "shardingKeys": [{
+ * "realm": "testRealm2",
+ * "shardingKey": "/sharding/key/1/f"
+ *}, {
+ * "realm": "testRealm2",
+ * "shardingKey": "/sharding/key/1/e"
+ * }, {
+ * "realm": "testRealm1",
+ * "shardingKey": "/sharding/key/1/b"
+ * }, {
+ * "realm": "testRealm1",
+ * "shardingKey": "/sharding/key/1/a"
+ * }]
+ * }
+ *
+ * - "HTTP GET /sharding-keys?realm={realm}&prefix={prefix}" which returns
sharding keys in the
+ * realm and that have the prefix.
*
- * @param realm Query param in endpoint path
- * @return Json representation of a map: shardingKeys -> collection of
sharding keys.
+ * @param realm Query param in endpoint path: metadata store realm.
+ * @param prefix Query param in endpoint path: prefix substring of sharding
key.
+ * @return Json representation for the sharding keys.
*/
@GET
@Path("/sharding-keys")
- public Response getShardingKeys(@QueryParam("realm") String realm) {
-Map responseMap;
-Collection shardingKeys;
+ public Response getShardingKeys(@QueryParam("realm") String realm,
Review comment:
A new endpoint which is equivalent to GET /sharding-keys?realm={realm} is
added: GET /metadata-store-realms/{realm}/sharding-keys
In my opinion, these 2 filters are valid for this resource. If remove one
query param, this method would only have 1 query param `prefix` and 1 if-else:
all sharding keys or sharding keys with prefix.
But again, I would like to offer this query param option as they are valid,
and it is also implemented now.
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 #761: Add REST read endpoints to helix-rest for metadata store directory
pkuwm commented on a change in pull request #761: Add REST read endpoints to
helix-rest for metadata store directory
URL: https://github.com/apache/helix/pull/761#discussion_r379283598
##
File path:
helix-rest/src/main/java/org/apache/helix/rest/server/resources/metadatastore/MetadataStoreDirectoryAccessor.java
##
@@ -111,41 +132,56 @@ public Response
deleteMetadataStoreRealm(@PathParam("realm") String realm) {
}
/**
- * Gets sharding keys mapped at path "HTTP GET /sharding-keys" which returns
all sharding keys in
- * a namespace, or path "HTTP GET /sharding-keys?realm={realmName}" which
returns sharding keys in
- * a realm.
+ * Gets all sharding keys mapped at paths:
+ * - "HTTP GET /sharding-keys" which returns all sharding keys in a
namespace.
+ * - "HTTP GET /sharding-keys?realm={realm}" which returns sharding keys in
the realm.
+ * - "HTTP GET /sharding-keys?prefix={prefix}" which returns sharding keys
that have the prefix.
+ * -- JSON response example for this path:
+ * {
+ * "prefix": "/sharding/key",
+ * "shardingKeys": [{
+ * "realm": "testRealm2",
+ * "shardingKey": "/sharding/key/1/f"
+ *}, {
+ * "realm": "testRealm2",
+ * "shardingKey": "/sharding/key/1/e"
+ * }, {
+ * "realm": "testRealm1",
+ * "shardingKey": "/sharding/key/1/b"
+ * }, {
+ * "realm": "testRealm1",
+ * "shardingKey": "/sharding/key/1/a"
+ * }]
+ * }
+ *
+ * - "HTTP GET /sharding-keys?realm={realm}&prefix={prefix}" which returns
sharding keys in the
+ * realm and that have the prefix.
*
- * @param realm Query param in endpoint path
- * @return Json representation of a map: shardingKeys -> collection of
sharding keys.
+ * @param realm Query param in endpoint path: metadata store realm.
+ * @param prefix Query param in endpoint path: prefix substring of sharding
key.
+ * @return Json representation for the sharding keys.
*/
@GET
@Path("/sharding-keys")
- public Response getShardingKeys(@QueryParam("realm") String realm) {
-Map responseMap;
-Collection shardingKeys;
+ public Response getShardingKeys(@QueryParam("realm") String realm,
+ @QueryParam("prefix") String prefix) {
try {
- // If realm is not set in query param, the endpoint is: "/sharding-keys"
- // to get all sharding keys in a namespace.
- if (realm == null) {
-shardingKeys = _metadataStoreDirectory.getAllShardingKeys(_namespace);
-// To avoid allocating unnecessary resource, limit the map's capacity
only for
-// SHARDING_KEYS.
-responseMap = new HashMap<>(1);
+ if (realm == null && prefix == null) {
+// For endpoint: "/sharding-keys" to get all sharding keys in a
namespace.
+return getAllShardingKeys();
+ } else if (prefix == null) {
+// For endpoint: "/sharding-keys?realm={realm}"
+return getAllShardingKeysInRealm(realm);
+ } else if (realm == null) {
+// For endpoint: "/sharding-keys?prefix={prefix}"
+return getAllShardingKeysUnderPath(prefix);
} else {
-// For endpoint: "/sharding-keys?realm={realmName}"
-shardingKeys =
_metadataStoreDirectory.getAllShardingKeysInRealm(_namespace, realm);
-// To avoid allocating unnecessary resource, limit the map's capacity
only for
-// SHARDING_KEYS and SINGLE_METADATA_STORE_REALM.
-responseMap = new HashMap<>(2);
-
responseMap.put(MetadataStoreRoutingConstants.SINGLE_METADATA_STORE_REALM,
realm);
+// For endpoint: "/sharding-keys?realm={realm}&prefix={prefix}"
+return getRealmShardingKeysUnderPath(realm, prefix);
Review comment:
A new endpoint which is equivalent to `GET /sharding-keys?realm={realm}` is
added: `GET /metadata-store-realms/{realm}/sharding-keys`
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 #761: Add REST read endpoints to helix-rest for metadata store directory
pkuwm commented on a change in pull request #761: Add REST read endpoints to
helix-rest for metadata store directory
URL: https://github.com/apache/helix/pull/761#discussion_r379282521
##
File path:
helix-rest/src/main/java/org/apache/helix/rest/server/resources/metadatastore/MetadataStoreDirectoryAccessor.java
##
@@ -66,24 +67,44 @@ private void postConstruct() {
}
/**
- * Gets all metadata store realms in a namespace with the endpoint.
+ * Gets all existing namespaces in the routing metadata store at endpoint:
+ * "GET /metadata-store-namespaces"
+ *
+ * @return Json response of all namespaces.
+ */
+ @GET
+ @Path("/metadata-store-namespaces")
+ public Response getAllNamespaces() {
+Collection namespaces = _metadataStoreDirectory.getAllNamespaces();
+Map> responseMap =
+
ImmutableMap.of(MetadataStoreRoutingConstants.METADATA_STORE_NAMESPACES,
namespaces);
+
+return JSONRepresentation(responseMap);
+ }
+
+ /**
+ * Gets all metadata store realms in a namespace at path: "GET
/metadata-store-realms",
+ * or gets a metadata store realm with the sharding key at path:
+ * "GET /metadata-store-realms?sharding-key={sharding-key}"
*
* @return Json representation of all realms.
*/
@GET
@Path("/metadata-store-realms")
- public Response getAllMetadataStoreRealms() {
-Map> responseMap;
+ public Response getAllMetadataStoreRealms(@QueryParam("sharding-key") String
shardingKey) {
Review comment:
`?sharding-key=` is a query param to filter the result. In this case, a
single object being returned is acceptable.
It is like
`/employees` returns all employees
`/employees?name=david` could returns a single employee (assume name is
unique).
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 #761: Add REST read endpoints to helix-rest for metadata store directory
pkuwm commented on a change in pull request #761: Add REST read endpoints to
helix-rest for metadata store directory
URL: https://github.com/apache/helix/pull/761#discussion_r379281125
##
File path:
helix-rest/src/main/java/org/apache/helix/rest/server/resources/metadatastore/MetadataStoreDirectoryAccessor.java
##
@@ -66,24 +67,44 @@ private void postConstruct() {
}
/**
- * Gets all metadata store realms in a namespace with the endpoint.
+ * Gets all existing namespaces in the routing metadata store at endpoint:
+ * "GET /metadata-store-namespaces"
+ *
+ * @return Json response of all namespaces.
+ */
+ @GET
+ @Path("/metadata-store-namespaces")
+ public Response getAllNamespaces() {
+Collection namespaces = _metadataStoreDirectory.getAllNamespaces();
+Map> responseMap =
+
ImmutableMap.of(MetadataStoreRoutingConstants.METADATA_STORE_NAMESPACES,
namespaces);
+
+return JSONRepresentation(responseMap);
+ }
+
+ /**
+ * Gets all metadata store realms in a namespace at path: "GET
/metadata-store-realms",
+ * or gets a metadata store realm with the sharding key at path:
+ * "GET /metadata-store-realms?sharding-key={sharding-key}"
Review comment:
I think the endpoint already tells the behavior. That's why we need to
follow the REST API design principal.
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 #761: Add REST read endpoints to helix-rest for metadata store directory
pkuwm commented on a change in pull request #761: Add REST read endpoints to
helix-rest for metadata store directory
URL: https://github.com/apache/helix/pull/761#discussion_r379283232
##
File path:
helix-rest/src/main/java/org/apache/helix/rest/server/resources/metadatastore/MetadataStoreDirectoryAccessor.java
##
@@ -66,24 +67,44 @@ private void postConstruct() {
}
/**
- * Gets all metadata store realms in a namespace with the endpoint.
+ * Gets all existing namespaces in the routing metadata store at endpoint:
+ * "GET /metadata-store-namespaces"
+ *
+ * @return Json response of all namespaces.
+ */
+ @GET
+ @Path("/metadata-store-namespaces")
+ public Response getAllNamespaces() {
+Collection namespaces = _metadataStoreDirectory.getAllNamespaces();
+Map> responseMap =
+
ImmutableMap.of(MetadataStoreRoutingConstants.METADATA_STORE_NAMESPACES,
namespaces);
+
+return JSONRepresentation(responseMap);
+ }
+
+ /**
+ * Gets all metadata store realms in a namespace at path: "GET
/metadata-store-realms",
+ * or gets a metadata store realm with the sharding key at path:
+ * "GET /metadata-store-realms?sharding-key={sharding-key}"
*
* @return Json representation of all realms.
*/
@GET
@Path("/metadata-store-realms")
- public Response getAllMetadataStoreRealms() {
-Map> responseMap;
+ public Response getAllMetadataStoreRealms(@QueryParam("sharding-key") String
shardingKey) {
Review comment:
My explanation above. It is OK to have if-else because we accept query
params. A better way to process query param is like we construct a query for DB:
```
query += param1,
query += param2
```
But we don't have a query layer here so if-else acts as this query layer.
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 #761: Add REST read endpoints to helix-rest for metadata store directory
pkuwm commented on a change in pull request #761: Add REST read endpoints to
helix-rest for metadata store directory
URL: https://github.com/apache/helix/pull/761#discussion_r379219384
##
File path:
helix-rest/src/main/java/org/apache/helix/rest/server/resources/metadatastore/MetadataStoreDirectoryAccessor.java
##
@@ -66,24 +67,44 @@ private void postConstruct() {
}
/**
- * Gets all metadata store realms in a namespace with the endpoint.
+ * Gets all existing namespaces in the routing metadata store at endpoint:
+ * "GET /metadata-store-namespaces"
+ *
+ * @return Json response of all namespaces.
+ */
+ @GET
+ @Path("/metadata-store-namespaces")
+ public Response getAllNamespaces() {
+Collection namespaces = _metadataStoreDirectory.getAllNamespaces();
+Map> responseMap =
+
ImmutableMap.of(MetadataStoreRoutingConstants.METADATA_STORE_NAMESPACES,
namespaces);
+
+return JSONRepresentation(responseMap);
+ }
+
+ /**
+ * Gets all metadata store realms in a namespace at path: "GET
/metadata-store-realms",
+ * or gets a metadata store realm with the sharding key at path:
+ * "GET /metadata-store-realms?sharding-key={sharding-key}"
*
* @return Json representation of all realms.
*/
@GET
@Path("/metadata-store-realms")
- public Response getAllMetadataStoreRealms() {
-Map> responseMap;
+ public Response getAllMetadataStoreRealms(@QueryParam("sharding-key") String
shardingKey) {
Review comment:
I've thought about this carefully. As it is necessary, I've defined a POJO
class `MetadataStoreShardingKey` to represent the object mapping. The response
will look like:
```
{
"metadataStoreRealms": ["realm-0", "realm-1"]
}
```
or single ream with sharding key:
```
{
"shardingKey": "/a/b/c",
"realm": "realm-0"
}
```
I definitely agree that if the response of object is complex (has more
fields), we would need POJO classes to represent the responses.
For the time being, since the most of the responses are quite simple: a
list, I don't think we want to make it complex . I've tried to make all json
responses to have key-value mappings.
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 #761: Add REST read endpoints to helix-rest for metadata store directory
pkuwm commented on a change in pull request #761: Add REST read endpoints to
helix-rest for metadata store directory
URL: https://github.com/apache/helix/pull/761#discussion_r379216863
##
File path:
helix-rest/src/main/java/org/apache/helix/rest/server/resources/metadatastore/MetadataStoreDirectoryAccessor.java
##
@@ -111,41 +132,56 @@ public Response
deleteMetadataStoreRealm(@PathParam("realm") String realm) {
}
/**
- * Gets sharding keys mapped at path "HTTP GET /sharding-keys" which returns
all sharding keys in
- * a namespace, or path "HTTP GET /sharding-keys?realm={realmName}" which
returns sharding keys in
- * a realm.
+ * Gets all sharding keys mapped at paths:
+ * - "HTTP GET /sharding-keys" which returns all sharding keys in a
namespace.
+ * - "HTTP GET /sharding-keys?realm={realm}" which returns sharding keys in
the realm.
+ * - "HTTP GET /sharding-keys?prefix={prefix}" which returns sharding keys
that have the prefix.
+ * -- JSON response example for this path:
+ * {
+ * "prefix": "/sharding/key",
+ * "shardingKeys": [{
+ * "realm": "testRealm2",
+ * "shardingKey": "/sharding/key/1/f"
+ *}, {
+ * "realm": "testRealm2",
+ * "shardingKey": "/sharding/key/1/e"
+ * }, {
+ * "realm": "testRealm1",
+ * "shardingKey": "/sharding/key/1/b"
+ * }, {
+ * "realm": "testRealm1",
+ * "shardingKey": "/sharding/key/1/a"
+ * }]
+ * }
+ *
+ * - "HTTP GET /sharding-keys?realm={realm}&prefix={prefix}" which returns
sharding keys in the
+ * realm and that have the prefix.
*
- * @param realm Query param in endpoint path
- * @return Json representation of a map: shardingKeys -> collection of
sharding keys.
+ * @param realm Query param in endpoint path: metadata store realm.
+ * @param prefix Query param in endpoint path: prefix substring of sharding
key.
+ * @return Json representation for the sharding keys.
*/
@GET
@Path("/sharding-keys")
- public Response getShardingKeys(@QueryParam("realm") String realm) {
-Map responseMap;
-Collection shardingKeys;
+ public Response getShardingKeys(@QueryParam("realm") String realm,
+ @QueryParam("prefix") String prefix) {
try {
- // If realm is not set in query param, the endpoint is: "/sharding-keys"
- // to get all sharding keys in a namespace.
- if (realm == null) {
-shardingKeys = _metadataStoreDirectory.getAllShardingKeys(_namespace);
-// To avoid allocating unnecessary resource, limit the map's capacity
only for
-// SHARDING_KEYS.
-responseMap = new HashMap<>(1);
+ if (realm == null && prefix == null) {
+// For endpoint: "/sharding-keys" to get all sharding keys in a
namespace.
+return getAllShardingKeys();
+ } else if (prefix == null) {
+// For endpoint: "/sharding-keys?realm={realm}"
+return getAllShardingKeysInRealm(realm);
+ } else if (realm == null) {
+// For endpoint: "/sharding-keys?prefix={prefix}"
+return getAllShardingKeysUnderPath(prefix);
} else {
-// For endpoint: "/sharding-keys?realm={realmName}"
-shardingKeys =
_metadataStoreDirectory.getAllShardingKeysInRealm(_namespace, realm);
-// To avoid allocating unnecessary resource, limit the map's capacity
only for
-// SHARDING_KEYS and SINGLE_METADATA_STORE_REALM.
-responseMap = new HashMap<>(2);
-
responseMap.put(MetadataStoreRoutingConstants.SINGLE_METADATA_STORE_REALM,
realm);
+// For endpoint: "/sharding-keys?realm={realm}&prefix={prefix}"
+return getRealmShardingKeysUnderPath(realm, prefix);
Review comment:
I've tried to make it as simple as possible. We have 3 different JAVA
methods in MetadataDirectory which accept different params.
https://github.com/apache/helix/blob/551ed63f98cf75b9953615386a0a9e1a9a566e33/helix-rest/src/main/java/org/apache/helix/rest/metadatastore/MetadataStoreDirectory.java#L52-L70
So even we set default values to the query params, we still have to check
the values and call the according method.
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
