[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9928 ) Change subject: IMPALA-2717: fix output of formatted unicode to non-TTY .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/9928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f Gerrit-Change-Number: 9928 Gerrit-PatchSet: 5 Gerrit-Owner: Tim ArmstrongGerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 12 Apr 2018 20:34:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9928 ) Change subject: IMPALA-2717: fix output of formatted unicode to non-TTY .. IMPALA-2717: fix output of formatted unicode to non-TTY The bug is that PrettyOutputFormatter.format() returned a unicode object, and Python cannot automatically write unicode objects to output streams where there is no default encoding. The fix is to convert to UTF-8 encoded in a regular string, which can be output to any output device. This makes the output type consistent with DelimitedOutputFormatter.format(). Based on code by Marcell Szabo. Testing: Added a basic test. Played around in an interactive shell to make sure that unicode characters still work in interactive mode. Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f Reviewed-on: http://gerrit.cloudera.org:8080/9928 Reviewed-by: Tim ArmstrongTested-by: Impala Public Jenkins --- M shell/impala_shell.py M shell/shell_output.py M tests/shell/test_shell_commandline.py 3 files changed, 31 insertions(+), 6 deletions(-) Approvals: Tim Armstrong: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/9928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f Gerrit-Change-Number: 9928 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9928 ) Change subject: IMPALA-2717: fix output of formatted unicode to non-TTY .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f Gerrit-Change-Number: 9928 Gerrit-PatchSet: 5 Gerrit-Owner: Tim ArmstrongGerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 12 Apr 2018 16:35:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9928 ) Change subject: IMPALA-2717: fix output of formatted unicode to non-TTY .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2292/ -- To view, visit http://gerrit.cloudera.org:8080/9928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f Gerrit-Change-Number: 9928 Gerrit-PatchSet: 5 Gerrit-Owner: Tim ArmstrongGerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 12 Apr 2018 16:35:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/9928 ) Change subject: IMPALA-2717: fix output of formatted unicode to non-TTY .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f Gerrit-Change-Number: 9928 Gerrit-PatchSet: 4 Gerrit-Owner: Tim ArmstrongGerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 12 Apr 2018 16:25:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9928 ) Change subject: IMPALA-2717: fix output of formatted unicode to non-TTY .. Patch Set 4: Could you have a look at this Mike? Would be good to validate that our use of Python strings is correct. -- To view, visit http://gerrit.cloudera.org:8080/9928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f Gerrit-Change-Number: 9928 Gerrit-PatchSet: 4 Gerrit-Owner: Tim ArmstrongGerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 11 Apr 2018 22:50:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9928 ) Change subject: IMPALA-2717: fix output of formatted unicode to non-TTY .. Patch Set 4: Code-Review+1 carry -- To view, visit http://gerrit.cloudera.org:8080/9928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f Gerrit-Change-Number: 9928 Gerrit-PatchSet: 4 Gerrit-Owner: Tim ArmstrongGerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 11 Apr 2018 22:50:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY
Hello Lars Volker, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9928 to look at the new patch set (#4). Change subject: IMPALA-2717: fix output of formatted unicode to non-TTY .. IMPALA-2717: fix output of formatted unicode to non-TTY The bug is that PrettyOutputFormatter.format() returned a unicode object, and Python cannot automatically write unicode objects to output streams where there is no default encoding. The fix is to convert to UTF-8 encoded in a regular string, which can be output to any output device. This makes the output type consistent with DelimitedOutputFormatter.format(). Based on code by Marcell Szabo. Testing: Added a basic test. Played around in an interactive shell to make sure that unicode characters still work in interactive mode. Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f --- M shell/impala_shell.py M shell/shell_output.py M tests/shell/test_shell_commandline.py 3 files changed, 31 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/9928/4 -- To view, visit http://gerrit.cloudera.org:8080/9928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f Gerrit-Change-Number: 9928 Gerrit-PatchSet: 4 Gerrit-Owner: Tim ArmstrongGerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9928 ) Change subject: IMPALA-2717: fix output of formatted unicode to non-TTY .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/9928/3/shell/shell_output.py File shell/shell_output.py: http://gerrit.cloudera.org:8080/#/c/9928/3/shell/shell_output.py@61 PS3, Line 61: > nit: remove space. Apologies for missing this one earlier. Done -- To view, visit http://gerrit.cloudera.org:8080/9928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f Gerrit-Change-Number: 9928 Gerrit-PatchSet: 3 Gerrit-Owner: Tim ArmstrongGerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 11 Apr 2018 22:50:05 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/9928 ) Change subject: IMPALA-2717: fix output of formatted unicode to non-TTY .. Patch Set 3: Code-Review+1 (1 comment) Thank you for fixing the open TODO, too! http://gerrit.cloudera.org:8080/#/c/9928/3/shell/shell_output.py File shell/shell_output.py: http://gerrit.cloudera.org:8080/#/c/9928/3/shell/shell_output.py@61 PS3, Line 61: nit: remove space. Apologies for missing this one earlier. -- To view, visit http://gerrit.cloudera.org:8080/9928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f Gerrit-Change-Number: 9928 Gerrit-PatchSet: 3 Gerrit-Owner: Tim ArmstrongGerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 11 Apr 2018 22:48:02 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9928 ) Change subject: IMPALA-2717: fix output of formatted unicode to non-TTY .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/9928/1/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/9928/1/shell/impala_shell.py@73 PS1, Line 73: """Patched version of PrettyTable that TODO""" > Not your change, but would you mind filling in the TODO? The difference see Hah oh dear. http://gerrit.cloudera.org:8080/#/c/9928/1/shell/shell_output.py File shell/shell_output.py: http://gerrit.cloudera.org:8080/#/c/9928/1/shell/shell_output.py@45 PS1, Line 45: row > I looked around a bit and tried to figure out how passing a str into add_ro I extended the current comment to explain the two-way conversion. http://gerrit.cloudera.org:8080/#/c/9928/2/shell/shell_output.py File shell/shell_output.py: http://gerrit.cloudera.org:8080/#/c/9928/2/shell/shell_output.py@31 PS2, Line 31: contained > nit: containing? Done http://gerrit.cloudera.org:8080/#/c/9928/2/shell/shell_output.py@58 PS2, Line 58: contained > here too? Done -- To view, visit http://gerrit.cloudera.org:8080/9928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f Gerrit-Change-Number: 9928 Gerrit-PatchSet: 2 Gerrit-Owner: Tim ArmstrongGerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 11 Apr 2018 21:57:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY
Hello Lars Volker, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9928 to look at the new patch set (#3). Change subject: IMPALA-2717: fix output of formatted unicode to non-TTY .. IMPALA-2717: fix output of formatted unicode to non-TTY The bug is that PrettyOutputFormatter.format() returned a unicode object, and Python cannot automatically write unicode objects to output streams where there is no default encoding. The fix is to convert to UTF-8 encoded in a regular string, which can be output to any output device. This makes the output type consistent with DelimitedOutputFormatter.format(). Based on code by Marcell Szabo. Testing: Added a basic test. Played around in an interactive shell to make sure that unicode characters still work in interactive mode. Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f --- M shell/impala_shell.py M shell/shell_output.py M tests/shell/test_shell_commandline.py 3 files changed, 31 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/9928/3 -- To view, visit http://gerrit.cloudera.org:8080/9928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f Gerrit-Change-Number: 9928 Gerrit-PatchSet: 3 Gerrit-Owner: Tim ArmstrongGerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/9928 ) Change subject: IMPALA-2717: fix output of formatted unicode to non-TTY .. Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/9928/1/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/9928/1/shell/impala_shell.py@73 PS1, Line 73: """Patched version of PrettyTable that TODO""" Not your change, but would you mind filling in the TODO? The difference seems to be that we're passing "replace" instead of "strict" in /usr/local/lib/python2.7/dist-packages/prettytable.py:181 . http://gerrit.cloudera.org:8080/#/c/9928/1/shell/shell_output.py File shell/shell_output.py: http://gerrit.cloudera.org:8080/#/c/9928/1/shell/shell_output.py@45 PS1, Line 45: row > It's already a str (verified by printing type(rows[0][0])). I looked around a bit and tried to figure out how passing a str into add_row() can result in unicode coming out of get_string(). PrettyTable calls _unicode() on the row's fields, which will encode the str() back into unicode. Do you want to add a brief comment mentioning this conversion inside add_row, maybe before L35? http://gerrit.cloudera.org:8080/#/c/9928/2/shell/shell_output.py File shell/shell_output.py: http://gerrit.cloudera.org:8080/#/c/9928/2/shell/shell_output.py@31 PS2, Line 31: containe nit: containing? http://gerrit.cloudera.org:8080/#/c/9928/2/shell/shell_output.py@58 PS2, Line 58: contained here too? -- To view, visit http://gerrit.cloudera.org:8080/9928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f Gerrit-Change-Number: 9928 Gerrit-PatchSet: 1 Gerrit-Owner: Tim ArmstrongGerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 11 Apr 2018 21:37:35 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9928 ) Change subject: IMPALA-2717: fix output of formatted unicode to non-TTY .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/9928/1/shell/shell_output.py File shell/shell_output.py: http://gerrit.cloudera.org:8080/#/c/9928/1/shell/shell_output.py@31 PS1, Line 31: > nit: remove space Done http://gerrit.cloudera.org:8080/#/c/9928/1/shell/shell_output.py@45 PS1, Line 45: row > should this be encoded in the same way? It's already a str (verified by printing type(rows[0][0])). I added a test for this code path too, since it is different. http://gerrit.cloudera.org:8080/#/c/9928/1/shell/shell_output.py@64 PS1, Line 64: rows > should this be "rows.encode(...)"? I checked that the returned rows is already a str instead of a unicode, so re-encoding it is not necessary. We already have test coverage for this case. -- To view, visit http://gerrit.cloudera.org:8080/9928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f Gerrit-Change-Number: 9928 Gerrit-PatchSet: 1 Gerrit-Owner: Tim ArmstrongGerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Apr 2018 18:31:09 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY
Hello Lars Volker, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9928 to look at the new patch set (#2). Change subject: IMPALA-2717: fix output of formatted unicode to non-TTY .. IMPALA-2717: fix output of formatted unicode to non-TTY The bug is that PrettyOutputFormatter.format() returned a unicode object, and Python cannot automatically write unicode objects to output streams where there is no default encoding. The fix is to convert to UTF-8 encoded in a regular string, which can be output to any output device. This makes the output type consistent with DelimitedOutputFormatter.format(). Based on code by Marcell Szabo. Testing: Added a basic test. Played around in an interactive shell to make sure that unicode characters still work in interactive mode. Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f --- M shell/shell_output.py M tests/shell/test_shell_commandline.py 2 files changed, 25 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/9928/2 -- To view, visit http://gerrit.cloudera.org:8080/9928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f Gerrit-Change-Number: 9928 Gerrit-PatchSet: 2 Gerrit-Owner: Tim ArmstrongGerrit-Reviewer: Lars Volker
[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/9928 ) Change subject: IMPALA-2717: fix output of formatted unicode to non-TTY .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/9928/1/shell/shell_output.py File shell/shell_output.py: http://gerrit.cloudera.org:8080/#/c/9928/1/shell/shell_output.py@31 PS1, Line 31: nit: remove space http://gerrit.cloudera.org:8080/#/c/9928/1/shell/shell_output.py@45 PS1, Line 45: row should this be encoded in the same way? http://gerrit.cloudera.org:8080/#/c/9928/1/shell/shell_output.py@64 PS1, Line 64: rows should this be "rows.encode(...)"? -- To view, visit http://gerrit.cloudera.org:8080/9928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f Gerrit-Change-Number: 9928 Gerrit-PatchSet: 1 Gerrit-Owner: Tim ArmstrongGerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Thu, 05 Apr 2018 01:19:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9928 Change subject: IMPALA-2717: fix output of formatted unicode to non-TTY .. IMPALA-2717: fix output of formatted unicode to non-TTY The bug is that PrettyOutputFormatter.format() returned a unicode object, and Python cannot automatically write unicode objects to output streams where there is no default encoding. The fix is to convert to UTF-8 encoded in a regular string, which can be output to any output device. This makes the output type consistent with DelimitedOutputFormatter.format(). Based on code by Marcell Szabo. Testing: Added a basic test. Played around in an interactive shell to make sure that unicode characters still work in interactive mode. Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f --- M shell/shell_output.py M tests/shell/test_shell_commandline.py 2 files changed, 16 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/9928/1 -- To view, visit http://gerrit.cloudera.org:8080/9928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I9de641ecf767a2feef3b9f48b344ef2d55e17a7f Gerrit-Change-Number: 9928 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong