Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 )
Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config ...................................................................... Patch Set 13: (3 comments) http://gerrit.cloudera.org:8080/#/c/13313/12/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/13313/12/tests/shell/test_shell_commandline.py@479 PS12, Line 479: def test_global_config_file(s > Removed it. Just curious why we don't need it though as the test below also Thanks for pointing that out, I looked into it and it seems like that was added because the previous version of that test invalidated metadata which would have affected other tests running in parallel. Since that is no longer true, can you please remove it from there as well? http://gerrit.cloudera.org:8080/#/c/13313/12/tests/shell/test_shell_commandline.py@487 PS12, Line 487: assert 'Query: select 2' in result.std > This is verifying that a valid global impalarc file should not trigger any What are the cases where warnings would be triggered? If there is a case specific to the global impalarc, then can you add the full warning text so that we have more context as to what we are asserting. If there is no specific warning message and you are sure nothing else will trigger a warning then in that case can you add an error text to it which is printed if this assert fails. For eg. assert 'WARNING:' not in result.stderr, "Error text that refers to what was expected but not encountered giving more context as to why this assert was added. also print the full result.stdout here so that it is easy to debug in case it fails" http://gerrit.cloudera.org:8080/#/c/13313/13/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/13313/13/tests/shell/test_shell_interactive.py@434 PS13, Line 434: v nit: upper case -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 13 Gerrit-Owner: Ethan Xue <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Ethan Xue <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Wed, 29 May 2019 20:57:12 +0000 Gerrit-HasComments: Yes
