You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Joe McDonnell (Code Review)" <ge...@cloudera.org> on 2022/05/23 22:43:45 UTC
[Impala-ASF-CR] IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18560
Change subject: IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
......................................................................
IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
Thrift 0.11.0 has known issues where Unicode errors are
not handler properly, including one case where the client
can hang. The traditional form factor for impala-shell
uses a patched Thrift that fixes those issues, but the
PyPi package uses the unpatched Thrift 0.11.0.
This modifies the requirements.txt file to use Thrift 0.14.2,
which has fixes for these Unicode issues. Thrift 0.14.2 has
a slightly different error message, so this amends the
allowed error messages in test_utf8_decoding_error_handling().
This is a bit awkward, given that the Python code generation
continues to happen with Thrift 0.11.0. Comparing the
Python code for Thrift 0.11 vs Thrift 0.14, I didn't see
noticeable differences. Given that the client can hang,
this seems worth fixing ahead of the full conversion to
Thrift 0.14 for all of Impala.
Testing:
- Ran the Unicode error handling tests with a PyPi
impala-shell
- Ran the shell tests normally
Change-Id: I63e0a5dda98df20c9184a347397118b1f3529603
(cherry picked from commit 468a26fdd8687e11d2e4b61946a84b5d8fef4e48)
---
M shell/packaging/requirements.txt
M tests/shell/test_shell_commandline.py
2 files changed, 6 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/18560/1
--
To view, visit http://gerrit.cloudera.org:8080/18560
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I63e0a5dda98df20c9184a347397118b1f3529603
Gerrit-Change-Number: 18560
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/18560 )
Change subject: IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18560/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18560/1//COMMIT_MSG@33
PS1, Line 33: 468a26fdd8687e11d2e4b61946a84b5d8fef4e48
> Is this a valid upstream commit id?
Oh, I forgot to remove that. I was cherrypicking on my dev machine, so this is not useful.
--
To view, visit http://gerrit.cloudera.org:8080/18560
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I63e0a5dda98df20c9184a347397118b1f3529603
Gerrit-Change-Number: 18560
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 24 May 2022 00:41:29 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/18560 )
Change subject: IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
......................................................................
Patch Set 2: Code-Review+2
Carry +2
--
To view, visit http://gerrit.cloudera.org:8080/18560
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I63e0a5dda98df20c9184a347397118b1f3529603
Gerrit-Change-Number: 18560
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 24 May 2022 00:47:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18560 )
Change subject: IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/10625/ : 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/18560
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I63e0a5dda98df20c9184a347397118b1f3529603
Gerrit-Change-Number: 18560
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Mon, 23 May 2022 23:03:42 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18560 )
Change subject: IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8155/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/18560
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I63e0a5dda98df20c9184a347397118b1f3529603
Gerrit-Change-Number: 18560
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 24 May 2022 16:44:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18560 )
Change subject: IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18560/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18560/1//COMMIT_MSG@33
PS1, Line 33: 468a26fdd8687e11d2e4b61946a84b5d8fef4e48
Is this a valid upstream commit id?
--
To view, visit http://gerrit.cloudera.org:8080/18560
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I63e0a5dda98df20c9184a347397118b1f3529603
Gerrit-Change-Number: 18560
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 24 May 2022 00:30:12 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18560
to look at the new patch set (#2).
Change subject: IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
......................................................................
IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
Thrift 0.11.0 has known issues where Unicode errors are
not handler properly, including one case where the client
can hang. The traditional form factor for impala-shell
uses a patched Thrift that fixes those issues, but the
PyPi package uses the unpatched Thrift 0.11.0.
This modifies the requirements.txt file to use Thrift 0.14.2,
which has fixes for these Unicode issues. Thrift 0.14.2 has
a slightly different error message, so this amends the
allowed error messages in test_utf8_decoding_error_handling().
This is a bit awkward, given that the Python code generation
continues to happen with Thrift 0.11.0. Comparing the
Python code for Thrift 0.11 vs Thrift 0.14, I didn't see
noticeable differences. Given that the client can hang,
this seems worth fixing ahead of the full conversion to
Thrift 0.14 for all of Impala.
Testing:
- Ran the Unicode error handling tests with a PyPi
impala-shell
- Ran the shell tests normally
Change-Id: I63e0a5dda98df20c9184a347397118b1f3529603
---
M shell/packaging/requirements.txt
M tests/shell/test_shell_commandline.py
2 files changed, 6 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/18560/2
--
To view, visit http://gerrit.cloudera.org:8080/18560
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I63e0a5dda98df20c9184a347397118b1f3529603
Gerrit-Change-Number: 18560
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
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/18560 )
Change subject: IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
......................................................................
IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
Thrift 0.11.0 has known issues where Unicode errors are
not handler properly, including one case where the client
can hang. The traditional form factor for impala-shell
uses a patched Thrift that fixes those issues, but the
PyPi package uses the unpatched Thrift 0.11.0.
This modifies the requirements.txt file to use Thrift 0.14.2,
which has fixes for these Unicode issues. Thrift 0.14.2 has
a slightly different error message, so this amends the
allowed error messages in test_utf8_decoding_error_handling().
This is a bit awkward, given that the Python code generation
continues to happen with Thrift 0.11.0. Comparing the
Python code for Thrift 0.11 vs Thrift 0.14, I didn't see
noticeable differences. Given that the client can hang,
this seems worth fixing ahead of the full conversion to
Thrift 0.14 for all of Impala.
Testing:
- Ran the Unicode error handling tests with a PyPi
impala-shell
- Ran the shell tests normally
Change-Id: I63e0a5dda98df20c9184a347397118b1f3529603
Reviewed-on: http://gerrit.cloudera.org:8080/18560
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M shell/packaging/requirements.txt
M tests/shell/test_shell_commandline.py
2 files changed, 6 insertions(+), 3 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/18560
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I63e0a5dda98df20c9184a347397118b1f3529603
Gerrit-Change-Number: 18560
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18560 )
Change subject: IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
......................................................................
Patch Set 1: Code-Review+1
This looks like it does what it says. Can't think of any reason this should be a problem, part of the rationale for things like Thrift is that different version clients can talk to each other.
--
To view, visit http://gerrit.cloudera.org:8080/18560
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I63e0a5dda98df20c9184a347397118b1f3529603
Gerrit-Change-Number: 18560
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Mon, 23 May 2022 22:47:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18560 )
Change subject: IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/18560
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I63e0a5dda98df20c9184a347397118b1f3529603
Gerrit-Change-Number: 18560
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 24 May 2022 21:09:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18560 )
Change subject: IMPALA-11313: Use Thrift 0.14.2 for impala-shell PyPi package
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/18560
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I63e0a5dda98df20c9184a347397118b1f3529603
Gerrit-Change-Number: 18560
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 24 May 2022 16:44:11 +0000
Gerrit-HasComments: No