Gabor Kaszab 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 14:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/15288/14/tests/custom_cluster/test_web_pages.py
File tests/custom_cluster/test_web_pages.py:

http://gerrit.cloudera.org:8080/#/c/15288/14/tests/custom_cluster/test_web_pages.py@113
PS14, Line 113: Create a long select query then check if it is contained in the 
response json.
nit: It would be more beneficial to describe the expected functionality here 
like: Check if the full query string is displayed in the query list on the 
WebUI.


http://gerrit.cloudera.org:8080/#/c/15288/14/tests/custom_cluster/test_web_pages.py@115
PS14, Line 115: "x " * 450
As this string is used multiple times I'd put this into a variable.


http://gerrit.cloudera.org:8080/#/c/15288/14/tests/custom_cluster/test_web_pages.py@115
PS14, Line 115: "select \"{0}\""
Might not work but could you try to put the string within single quotes instead 
of double quotes so that you don't have to escape the double quotes inside?


http://gerrit.cloudera.org:8080/#/c/15288/14/tests/custom_cluster/test_web_pages.py@116
PS14, Line 116: theere
typo


http://gerrit.cloudera.org:8080/#/c/15288/14/tests/custom_cluster/test_web_pages.py@118
PS14, Line 118:     expected = "select \\\"{0}\\\"".format("x " * 450)
Same comment for single quotes as above.


http://gerrit.cloudera.org:8080/#/c/15288/14/tests/custom_cluster/test_web_pages.py@129
PS14, Line 129:     """Create a long select query then check if it is contained 
in the response json."""
This statements isn't true for this test. Might be a copy-paste error.


http://gerrit.cloudera.org:8080/#/c/15288/14/tests/custom_cluster/test_web_pages.py@131
PS14, Line 131:     query = "select \"{0}\"".format("x " * 450)
Same comment for single quotes as above.


http://gerrit.cloudera.org:8080/#/c/15288/14/tests/custom_cluster/test_web_pages.py@133
PS14, Line 133: theere
typo


http://gerrit.cloudera.org:8080/#/c/15288/14/tests/custom_cluster/test_web_pages.py@135
PS14, Line 135:     expected = "select \\\"x ..."
Same comment for single quotes as above.


http://gerrit.cloudera.org:8080/#/c/15288/14/tests/custom_cluster/test_web_pages.py@139
PS14, Line 139: if unexpected in response_json:
              :       assert False, "The response is containing the full query."
Is this needed? L141 will check the expected output anyway that won't be a 
prefix of the input because of the "..." appended to the end.



-- 
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: 14
Gerrit-Owner: Adam Tamas <ta...@cloudera.com>
Gerrit-Reviewer: Adam Tamas <ta...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <norbert.lu...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 16 Mar 2020 12:55:45 +0000
Gerrit-HasComments: Yes

Reply via email to