Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15288 )

Change subject: IMPALA-6360: Don't show full query statement on Impala webUI by 
default. Added the ‘query_stmt_size’ flag to impala-server.cc with default 
value of 250 and modified the ‘ImpalaHttpHandler::QueryStateToJson()’ to 
truncate the end of the statements if they
......................................................................


Patch Set 4:

(4 comments)

Hi Tamas, thank you for the change.
Just a few nits.

http://gerrit.cloudera.org:8080/#/c/15288/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15288/4//COMMIT_MSG@7
PS4, Line 7: IMPALA-6360: Don't show full query statement on Impala webUI by 
default.
           : Added the ‘query_stmt_size’ flag to impala-server.cc with default 
value
nit: missing blank line between subject and body
nit: period is not needed at the end of subject


http://gerrit.cloudera.org:8080/#/c/15288/4/be/src/service/impala-http-handler.cc
File be/src/service/impala-http-handler.cc:

http://gerrit.cloudera.org:8080/#/c/15288/4/be/src/service/impala-http-handler.cc@374
PS4, Line 374:   Value stmt((FLAGS_query_stmt_size) ? (tmp_stmt.length() > 
FLAGS_query_stmt_size) ?
             :       tmp_stmt.substr(0, 
FLAGS_query_stmt_size).append("...").c_str() : tmp_stmt.c_str() :
             :       tmp_stmt.c_str(), document->GetAllocator());
This is a bit hard to read at first, if we would change the first ternary 
operator to if then the code would better document itself and the comment could 
be removed, similar to line 401 in this file.


http://gerrit.cloudera.org:8080/#/c/15288/4/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/15288/4/be/src/service/impala-server.cc@151
PS4, Line 151: has
nit: have


http://gerrit.cloudera.org:8080/#/c/15288/4/tests/webserver/test_web_pages.py
File tests/webserver/test_web_pages.py:

http://gerrit.cloudera.org:8080/#/c/15288/4/tests/webserver/test_web_pages.py@425
PS4, Line 425: lacus at risus bibendum, id pulvinar ligula lobortis. Fusce 
lacinia nibh in
             : volutpat iaculis. Cras vite dignissim ligula. Fusce 
sollici.Proin bibendum erat
             : eu libero iaculis pharetra. Duis efficitur lacus at risus 
bibendum, id pulvinar
             : ligula lobortis. Fusce lacinia nibh in volutpat iaculis. Cras 
vite dignissim
             : ligula. Fusce sollici.\""""
nit: missing identation



--
To view, visit http://gerrit.cloudera.org:8080/15288
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7109a0be5d1022b4f8d6e72441cf5dc1dc42605
Gerrit-Change-Number: 15288
Gerrit-PatchSet: 4
Gerrit-Owner: Adam Tamas <[email protected]>
Gerrit-Reviewer: Gabor Kaszab <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Tamas Mate <[email protected]>
Gerrit-Comment-Date: Thu, 27 Feb 2020 08:41:03 +0000
Gerrit-HasComments: Yes

Reply via email to