Adam Tamas 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
......................................................................


Patch Set 4:

(6 comments)

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
Done


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 o
Done


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

http://gerrit.cloudera.org:8080/#/c/15288/5/be/src/service/impala-http-handler.cc@371
PS5, Line 371:   /*if the statement is longer than the allowed query_stmt_size 
then this will cut it and
             :     put "..." in the end.
             :   */
> I believe the bellow code part is descriptive enough and documents itself,
Done


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
Done


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
Done


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

http://gerrit.cloudera.org:8080/#/c/15288/5/tests/webserver/test_web_pages.py@421
PS5, Line 421: to a 'lorem ipsum'
> nit: I think there is no need to share this information, this part could be
Done



--
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: 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: Mon, 02 Mar 2020 12:25:53 +0000
Gerrit-HasComments: Yes

Reply via email to