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 3: (10 comments) Patch 3 should address most issues. I will figure out how to attach screenshot tomorrow. Can we guarantee the test queries always go to impalad with port 25000? http://gerrit.cloudera.org:8080/#/c/13333/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13333/1//COMMIT_MSG@9 PS1, Line 9: Add a link called "Download Text Profile" to profile tab. > Suggest adding a few more things; mention that the profile tab now includes Done http://gerrit.cloudera.org:8080/#/c/13333/1/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/13333/1/tests/webserver/test_web_pages.py@546 PS1, Line 546: > should we add another test similar to test_query_profile_encoded_unknown_qu Add to the end of the test. http://gerrit.cloudera.org:8080/#/c/13333/1/tests/webserver/test_web_pages.py@549 PS1, Line 549: query = "select count(*) from functional.alltypes" > is there a race condition if this is executed asynchronously? what if the r Done http://gerrit.cloudera.org:8080/#/c/13333/1/tests/webserver/test_web_pages.py@553 PS1, Line 553: > don't mix + and %s python formatting. I think we should use "{0}query_profi Done http://gerrit.cloudera.org:8080/#/c/13333/1/tests/webserver/test_web_pages.py@554 PS1, Line 554: id={1}" > use IMPALAD_TEST_PORT Done http://gerrit.cloudera.org:8080/#/c/13333/1/tests/webserver/test_web_pages.py@555 PS1, Line 555: self.ROOT_URL, query_h > I'm no Javascript expert, so if this approach is getting too complex. I thi Done http://gerrit.cloudera.org:8080/#/c/13333/1/www/query_profile.tmpl File www/query_profile.tmpl: http://gerrit.cloudera.org:8080/#/c/13333/1/www/query_profile.tmpl@34 PS1, Line 34: <h4> > is download the name of the file that will be downloaded? if so, I suggest Done http://gerrit.cloudera.org:8080/#/c/13333/1/www/query_profile.tmpl@43 PS1, Line 43: <script> > I think it would make more sense to use getElementById instead of getElemen Done http://gerrit.cloudera.org:8080/#/c/13333/1/www/query_profile.tmpl@47 PS1, Line 47: > nit: join this to L46 Done http://gerrit.cloudera.org:8080/#/c/13333/1/www/query_profile.tmpl@49 PS1, Line 49: > nit: txtprofile -> text_profile 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: 3 Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com> Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com> Gerrit-Comment-Date: Wed, 15 May 2019 03:33:33 +0000 Gerrit-HasComments: Yes
