Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/13333 )
Change subject: IMPALA-6903: Download profile from WebUI in text format ...................................................................... Patch Set 6: (7 comments) http://gerrit.cloudera.org:8080/#/c/13333/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13333/3//COMMIT_MSG@10 PS3, Line 10: The link allows users to download runtime profiles as UTF-8 > nit: this line should be wrapped Done http://gerrit.cloudera.org:8080/#/c/13333/3//COMMIT_MSG@14 PS3, Line 14: Tests: > Typically you want to include something like "ran all core tests" Done http://gerrit.cloudera.org:8080/#/c/13333/3/be/src/service/impala-http-handler.h File be/src/service/impala-http-handler.h: http://gerrit.cloudera.org:8080/#/c/13333/3/be/src/service/impala-http-handler.h@119 PS3, Line 119: rapidjson::Document* document); > nit: not sure if "Plain" is the correct descriptor here, its a big vague wh Done http://gerrit.cloudera.org:8080/#/c/13333/3/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/13333/3/tests/webserver/test_web_pages.py@551 PS3, Line 551: try: > Use I need no timeout. http://gerrit.cloudera.org:8080/#/c/13333/3/tests/webserver/test_web_pages.py@563 PS3, Line 563: assert download_link in responses[0].text > do we need a null check here - e.g. if responses[0].text is null Even the impalad does not run the query, the response is not null. http://gerrit.cloudera.org:8080/#/c/13333/3/tests/webserver/test_web_pages.py@568 PS3, Line 568: # Check the query id is in the content of the reponse. > self.ROOT_URL.format(download_link) + is right, the {0} in ROOT_URL is for port number. http://gerrit.cloudera.org:8080/#/c/13333/3/tests/webserver/test_web_pages.py@572 PS3, Line 572: self.client.close_query(query_handle) > this should be in a "finally" block Done -- To view, visit http://gerrit.cloudera.org:8080/13333 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie030c2bb330211f51840417b9f7880f19174af7b Gerrit-Change-Number: 13333 Gerrit-PatchSet: 6 Gerrit-Owner: Yongzhi Chen <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Sahil Takiar <[email protected]> Gerrit-Reviewer: Yongzhi Chen <[email protected]> Gerrit-Comment-Date: Wed, 22 May 2019 02:02:29 +0000 Gerrit-HasComments: Yes
