Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19893 )
Change subject: IMPALA-12145: Fix profiles with non-ascii character in impala-shell (python2) ...................................................................... Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/19893/3/shell/shell_output.py File shell/shell_output.py: http://gerrit.cloudera.org:8080/#/c/19893/3/shell/shell_output.py@32 PS3, Line 32: def match_string_type(str_to_convert, reference_str): > The reference_str looks a bit weird when looking at a use of this function, I couldn't think of a better name (open to suggestions) http://gerrit.cloudera.org:8080/#/c/19893/3/shell/shell_output.py@32 PS3, Line 32: : I don't know about unit tests for the impala-shell - in tests/shell the tests are calling impala-shell through the command line. I agree that it would be good to have unit test that call the Python functions directly instead of the current end-to-end tests. Is it ok to do the unit test in another commit? As it would the first of its kind, it may be better to have some larger discussion about where to put the test and I don't want to block this simple fix with it. -- To view, visit http://gerrit.cloudera.org:8080/19893 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I99b72dd262fc7c382e8baee1dce7592880c84de2 Gerrit-Change-Number: 19893 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Comment-Date: Mon, 22 May 2023 14:58:49 +0000 Gerrit-HasComments: Yes
