You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Andrew Sherman (Code Review)" <ge...@cloudera.org> on 2021/01/20 21:25:06 UTC
[Impala-ASF-CR] IMPALA-10447: Add a newline when exporting shell output to a file.
Andrew Sherman has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16966
Change subject: IMPALA-10447: Add a newline when exporting shell output to a file.
......................................................................
IMPALA-10447: Add a newline when exporting shell output to a file.
Impala shell outputs a batch of rows using OutputStream. Inside
OutputStream, output to a file is handled slightly differently from
output that is written to stdout. When writing to stdout we use print()
(which appends a newline) while when writing to a file we use write()
(which adds nothing). This difference was introduced in IMPALA-3343 so
this bug may be a regression introduced then. To ensure that output is
the same in either case we need to add a newline after writing each
batch of rows to a file.
TESTING:
Added a new test for this case.
Change-Id: I078a06c54e0834bc1f898626afbfff4ded579fa9
---
M shell/shell_output.py
M tests/shell/test_shell_commandline.py
2 files changed, 30 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/66/16966/1
--
To view, visit http://gerrit.cloudera.org:8080/16966
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I078a06c54e0834bc1f898626afbfff4ded579fa9
Gerrit-Change-Number: 16966
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
[Impala-ASF-CR] IMPALA-10447: Add a newline when exporting shell output to a file.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16966 )
Change subject: IMPALA-10447: Add a newline when exporting shell output to a file.
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6856/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/16966
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I078a06c54e0834bc1f898626afbfff4ded579fa9
Gerrit-Change-Number: 16966
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 26 Jan 2021 02:56:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10447: Add a newline when exporting shell output to a file.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16966 )
Change subject: IMPALA-10447: Add a newline when exporting shell output to a file.
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/16966
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I078a06c54e0834bc1f898626afbfff4ded579fa9
Gerrit-Change-Number: 16966
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 26 Jan 2021 08:32:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10447: Add a newline when exporting shell output to a file.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16966 )
Change subject: IMPALA-10447: Add a newline when exporting shell output to a file.
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8016/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/16966
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I078a06c54e0834bc1f898626afbfff4ded579fa9
Gerrit-Change-Number: 16966
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sun, 24 Jan 2021 17:26:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10447: Add a newline when exporting shell output to a file.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16966 )
Change subject: IMPALA-10447: Add a newline when exporting shell output to a file.
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/16966
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I078a06c54e0834bc1f898626afbfff4ded579fa9
Gerrit-Change-Number: 16966
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 26 Jan 2021 02:56:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10447: Add a newline when exporting shell output to a file.
Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
Andrew Sherman has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/16966 )
Change subject: IMPALA-10447: Add a newline when exporting shell output to a file.
......................................................................
IMPALA-10447: Add a newline when exporting shell output to a file.
Impala shell outputs a batch of rows using OutputStream. Inside
OutputStream, output to a file is handled slightly differently from
output that is written to stdout. When writing to stdout we use print()
(which appends a newline) while when writing to a file we use write()
(which adds nothing). This difference was introduced in IMPALA-3343 so
this bug may be a regression introduced then. To ensure that output is
the same in either case we need to add a newline after writing each
batch of rows to a file.
TESTING:
Added a new test for this case.
Change-Id: I078a06c54e0834bc1f898626afbfff4ded579fa9
---
M shell/shell_output.py
M tests/shell/test_shell_commandline.py
2 files changed, 29 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/66/16966/4
--
To view, visit http://gerrit.cloudera.org:8080/16966
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I078a06c54e0834bc1f898626afbfff4ded579fa9
Gerrit-Change-Number: 16966
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-10447: Add a newline when exporting shell output to a file.
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/16966 )
Change subject: IMPALA-10447: Add a newline when exporting shell output to a file.
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/16966
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I078a06c54e0834bc1f898626afbfff4ded579fa9
Gerrit-Change-Number: 16966
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 25 Jan 2021 23:36:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10447: Add a newline when exporting shell output to a file.
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/16966 )
Change subject: IMPALA-10447: Add a newline when exporting shell output to a file.
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
Thanks for fixing this quickly! LGTM, just have a minor question.
http://gerrit.cloudera.org:8080/#/c/16966/3/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/16966/3/tests/shell/test_shell_commandline.py@1089
PS3, Line 1089: 'i_item_sk'
Is it intended to make this a constant string literal?
--
To view, visit http://gerrit.cloudera.org:8080/16966
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I078a06c54e0834bc1f898626afbfff4ded579fa9
Gerrit-Change-Number: 16966
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 25 Jan 2021 02:52:03 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10447: Add a newline when exporting shell output to a file.
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/16966 )
Change subject: IMPALA-10447: Add a newline when exporting shell output to a file.
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16966/1/shell/shell_output.py
File shell/shell_output.py:
http://gerrit.cloudera.org:8080/#/c/16966/1/shell/shell_output.py@120
PS1, Line 120: '\n'
This should be b'\n'. Python3 requires this arg be in 'bytes' instead of 'str' since we open the file using binary mode ('b').
$ python3
Python 3.7.5 (default, Nov 1 2019, 02:16:32)
[Clang 11.0.0 (clang-1100.0.33.8)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> with open('/tmp/my.txt', 'ab') as out_file:
... out_file.write('\n')
...
Traceback (most recent call last):
File "<stdin>", line 2, in <module>
TypeError: a bytes-like object is required, not 'str'
>>> with open('/tmp/my.txt', 'ab') as out_file:
... out_file.write(b'\n')
...
1
>>>
--
To view, visit http://gerrit.cloudera.org:8080/16966
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I078a06c54e0834bc1f898626afbfff4ded579fa9
Gerrit-Change-Number: 16966
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 22 Jan 2021 08:58:22 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10447: Add a newline when exporting shell output to a file.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16966 )
Change subject: IMPALA-10447: Add a newline when exporting shell output to a file.
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8021/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/16966
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I078a06c54e0834bc1f898626afbfff4ded579fa9
Gerrit-Change-Number: 16966
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 25 Jan 2021 19:39:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10447: Add a newline when exporting shell output to a file.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16966 )
Change subject: IMPALA-10447: Add a newline when exporting shell output to a file.
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16966/1/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/16966/1/tests/shell/test_shell_commandline.py@1102
PS1, Line 1102:
flake8: W391 blank line at end of file
--
To view, visit http://gerrit.cloudera.org:8080/16966
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I078a06c54e0834bc1f898626afbfff4ded579fa9
Gerrit-Change-Number: 16966
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 Jan 2021 21:25:57 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10447: Add a newline when exporting shell output to a file.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16966 )
Change subject: IMPALA-10447: Add a newline when exporting shell output to a file.
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8031/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/16966
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I078a06c54e0834bc1f898626afbfff4ded579fa9
Gerrit-Change-Number: 16966
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 Jan 2021 21:47:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10447: Add a newline when exporting shell output to a file.
Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
Andrew Sherman has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/16966 )
Change subject: IMPALA-10447: Add a newline when exporting shell output to a file.
......................................................................
IMPALA-10447: Add a newline when exporting shell output to a file.
Impala shell outputs a batch of rows using OutputStream. Inside
OutputStream, output to a file is handled slightly differently from
output that is written to stdout. When writing to stdout we use print()
(which appends a newline) while when writing to a file we use write()
(which adds nothing). This difference was introduced in IMPALA-3343 so
this bug may be a regression introduced then. To ensure that output is
the same in either case we need to add a newline after writing each
batch of rows to a file.
TESTING:
Added a new test for this case.
Change-Id: I078a06c54e0834bc1f898626afbfff4ded579fa9
---
M shell/shell_output.py
M tests/shell/test_shell_commandline.py
2 files changed, 29 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/66/16966/3
--
To view, visit http://gerrit.cloudera.org:8080/16966
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I078a06c54e0834bc1f898626afbfff4ded579fa9
Gerrit-Change-Number: 16966
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-10447: Add a newline when exporting shell output to a file.
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/16966 )
Change subject: IMPALA-10447: Add a newline when exporting shell output to a file.
......................................................................
IMPALA-10447: Add a newline when exporting shell output to a file.
Impala shell outputs a batch of rows using OutputStream. Inside
OutputStream, output to a file is handled slightly differently from
output that is written to stdout. When writing to stdout we use print()
(which appends a newline) while when writing to a file we use write()
(which adds nothing). This difference was introduced in IMPALA-3343 so
this bug may be a regression introduced then. To ensure that output is
the same in either case we need to add a newline after writing each
batch of rows to a file.
TESTING:
Added a new test for this case.
Change-Id: I078a06c54e0834bc1f898626afbfff4ded579fa9
Reviewed-on: http://gerrit.cloudera.org:8080/16966
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M shell/shell_output.py
M tests/shell/test_shell_commandline.py
2 files changed, 29 insertions(+), 0 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/16966
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I078a06c54e0834bc1f898626afbfff4ded579fa9
Gerrit-Change-Number: 16966
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>