You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/05/11 17:32:40 UTC
[GitHub] [incubator-pinot] npawar opened a new pull request #6900: TableConfigs toJsonObject fix
npawar opened a new pull request #6900:
URL: https://github.com/apache/incubator-pinot/pull/6900
Making this backward compatible with how the `GET schema` call is.
Without this fix, the `GET tableConfigs` call returns the fields like "defaultNullValue", "maxLength" etc, even if user hadn't set them while creating the config. This interferes with the update call.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] npawar commented on a change in pull request #6900: TableConfigs toJsonObject fix
Posted by GitBox <gi...@apache.org>.
npawar commented on a change in pull request #6900:
URL: https://github.com/apache/incubator-pinot/pull/6900#discussion_r630432615
##########
File path: pinot-spi/src/main/java/org/apache/pinot/spi/config/TableConfigs.java
##########
@@ -70,6 +71,24 @@ public TableConfig getRealtime() {
return _realtime;
}
+ @Override
+ public String toJsonString() {
+ try {
+ ObjectNode tableConfigs = JsonUtils.newObjectNode();
+ tableConfigs.put("tableName", _tableName);
+ tableConfigs.set("schema", _schema.toJsonObject());
+ if (_offline != null) {
+ tableConfigs.set("offline", _offline.toJsonNode());
+ }
+ if (_realtime != null) {
+ tableConfigs.set("realtime", _realtime.toJsonNode());
+ }
+ return JsonUtils.objectToString(tableConfigs);
+ } catch (JsonProcessingException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
public String toPrettyJsonString() {
Review comment:
yes, good catch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6900: TableConfigs toJsonObject fix
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6900:
URL: https://github.com/apache/incubator-pinot/pull/6900#discussion_r630412433
##########
File path: pinot-spi/src/main/java/org/apache/pinot/spi/config/TableConfigs.java
##########
@@ -70,6 +71,24 @@ public TableConfig getRealtime() {
return _realtime;
}
+ @Override
+ public String toJsonString() {
+ try {
+ ObjectNode tableConfigs = JsonUtils.newObjectNode();
+ tableConfigs.put("tableName", _tableName);
+ tableConfigs.set("schema", _schema.toJsonObject());
+ if (_offline != null) {
+ tableConfigs.set("offline", _offline.toJsonNode());
+ }
+ if (_realtime != null) {
+ tableConfigs.set("realtime", _realtime.toJsonNode());
+ }
+ return JsonUtils.objectToString(tableConfigs);
+ } catch (JsonProcessingException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
public String toPrettyJsonString() {
Review comment:
Should we also modify this method? We can add a helper method `toJsonObject()` and both of them can use that
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] npawar merged pull request #6900: TableConfigs toJsonObject fix
Posted by GitBox <gi...@apache.org>.
npawar merged pull request #6900:
URL: https://github.com/apache/incubator-pinot/pull/6900
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org