Jackie-Jiang commented on code in PR #8606:
URL: https://github.com/apache/pinot/pull/8606#discussion_r861406857
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSchemaRestletResource.java:
##########
@@ -145,16 +148,19 @@ public SuccessResponse deleteSchema(
@Authenticate(AccessType.UPDATE)
@ApiOperation(value = "Update a schema", notes = "Updates a schema")
@ApiResponses(value = {
- @ApiResponse(code = 200, message = "Successfully updated schema"),
- @ApiResponse(code = 404, message = "Schema not found"),
- @ApiResponse(code = 400, message = "Missing or invalid request body"),
- @ApiResponse(code = 500, message = "Internal error")
+ @ApiResponse(code = 200, message = "Successfully updated schema"),
@ApiResponse(code = 404, message = "Schema "
Review Comment:
Don't change this format (if you apply the latest checkstyle rules, it
should not reformat this)
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSchemaRestletResource.java:
##########
@@ -243,13 +276,26 @@ public String validateSchema(FormDataMultiPart multiPart)
{
@ApiOperation(value = "Validate schema", notes = "This API returns the
schema that matches the one you get "
+ "from 'GET /schema/{schemaName}'. This allows us to validate schema
before apply.")
@ApiResponses(value = {
- @ApiResponse(code = 200, message = "Successfully validated schema"),
- @ApiResponse(code = 400, message = "Missing or invalid request body"),
- @ApiResponse(code = 500, message = "Internal error")
+ @ApiResponse(code = 200, message = "Successfully validated schema"),
@ApiResponse(code = 400, message =
+ "Missing or invalid request body"), @ApiResponse(code = 500, message =
"Internal error")
})
- public String validateSchema(Schema schema) {
+ public String validateSchema(String schemaJsonString) {
+ Pair<Schema, Map<String, Object>> schemaAndUnrecognizedProps = null;
+ try {
+ schemaAndUnrecognizedProps =
JsonUtils.stringToObjectAndUnrecognizedProperties(schemaJsonString,
Schema.class);
+ } catch (Exception e) {
+ String msg = String.format("Invalid schema config json string: %s",
schemaJsonString);
+ throw new ControllerApplicationException(LOGGER, msg,
Response.Status.BAD_REQUEST, e);
+ }
+ Schema schema = schemaAndUnrecognizedProps.getLeft();
validateSchemaInternal(schema);
- return schema.toPrettyJsonString();
+ ObjectNode response = schema.toJsonObject();
+ response.set("unrecognizedProperties",
JsonUtils.objectToJsonNode(schemaAndUnrecognizedProps.getRight()));
+ try {
+ return JsonUtils.objectToPrettyString(response);
Review Comment:
(minor) I think you can directly return the `ObjectNode`
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java:
##########
@@ -147,16 +150,18 @@ public String getConfig(
@POST
@Produces(MediaType.APPLICATION_JSON)
@Path("/tableConfigs")
- @ApiOperation(value = "Add the TableConfigs using the tableConfigsStr json",
- notes = "Add the TableConfigs using the tableConfigsStr json")
- public SuccessResponse addConfig(
- String tableConfigsStr,
+ @ApiOperation(value = "Add the TableConfigs using the tableConfigsStr json",
notes = "Add the TableConfigs using "
Review Comment:
(code style) Same here, don't change this format because it doesn't improve
readability
--
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.
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
