Todd Lipcon has posted comments on this change. Change subject: KUDU-1388: Master web UI should show table's partition schema ......................................................................
Patch Set 2: (3 comments) looks like there's a LINT error too - click through to the build and you should be able to download the lint log from the failure and see what's wrong. http://gerrit.cloudera.org:8080/#/c/3022/2/src/kudu/common/partition.cc File src/kudu/common/partition.cc: Line 639: vector<string> display_strings; why use a vector<string> instead of just using a string here, and then use .append() or SubstituteAndAppend() below? http://gerrit.cloudera.org:8080/#/c/3022/2/src/kudu/common/partition.h File src/kudu/common/partition.h: Line 177: std::string DebugString(const Schema& schema) const; hrm, now I see we already have DebugString(). Is that format just too ugly to put on our web UI? http://gerrit.cloudera.org:8080/#/c/3022/2/src/kudu/master/master-path-handlers.cc File src/kudu/master/master-path-handlers.cc: Line 195: *output << partition_schema.DisplayString(schema); even if it's got <pre> around it, we should EscapeForHtmlToString to avoid injection attacks -- To view, visit http://gerrit.cloudera.org:8080/3022 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I81b634f6dd51e63205bbd7956273b74ca63459f7 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
