You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2018/04/04 22:27:34 UTC

[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 <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
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 Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Apr 2018 22:48:02 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
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 Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Apr 2018 21:37:35 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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 Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Apr 2018 21:57:20 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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 Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Apr 2018 22:50:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Apr 2018 16:35:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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 Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>

[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY

Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
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 Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Apr 2018 16:25:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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 Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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 Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Apr 2018 22:50:05 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
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 Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Apr 2018 01:19:32 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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 Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Apr 2018 16:35:32 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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 Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Apr 2018 18:31:09 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
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 <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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 Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Apr 2018 22:50:51 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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 Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-2717: fix output of formatted unicode to non-TTY

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Apr 2018 20:34:45 +0000
Gerrit-HasComments: No