Ian Maxon has posted comments on this change.

Change subject: [UI,SITE] Allow logical plan to be viewed as JSON / formatted 
JSON
......................................................................


Patch Set 9:

(4 comments)

Few minor nits. Other than that, looks great, pretty much there IMO.

https://asterix-gerrit.ics.uci.edu/#/c/1885/9/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ApiServlet.java
File 
asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ApiServlet.java:

Line 123:             planFormat = PlanFormat.valueOf(plan);
> CRITICAL SonarQube violation:
Hm, this seems like it'd be good to fix somehow.


https://asterix-gerrit.ics.uci.edu/#/c/1885/9/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/jsonplan/JsonLogicalPlanTest.java
File 
asterixdb/asterix-app/src/test/java/org/apache/asterix/test/jsonplan/JsonLogicalPlanTest.java:

PS9, Line 61: 
Seems like this is more or less the same as its optimized counterpart sans a 
few variables, can we just make one extend the other to avoid duplicating 
everything?


https://asterix-gerrit.ics.uci.edu/#/c/1885/9/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/jsonplan/JsonOptimizedLogicalPlanTest.java
File 
asterixdb/asterix-app/src/test/java/org/apache/asterix/test/jsonplan/JsonOptimizedLogicalPlanTest.java:

PS9, Line 197: boolean firstPlan = false;
Whats this about?


PS9, Line 211: final JsonParser parser = new 
ObjectMapper().getJsonFactory().createJsonParser(objectActual);
             :                 while (parser.nextToken() != null) {
             :                 }
So you just want to check if it's a valid JSON object?


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1885
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4dd62e355048a5b8a02e074049fe41e73e74e357
Gerrit-PatchSet: 9
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: shi...@uci.edu
Gerrit-Reviewer: Ian Maxon <ima...@apache.org>
Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-HasComments: Yes

Reply via email to