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 2023/04/29 00:16:40 UTC
[Impala-ASF-CR] IMPALA-12059: Filter out harmless warnings from Thrift compilation
Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19818
Change subject: IMPALA-12059: Filter out harmless warnings from Thrift compilation
......................................................................
IMPALA-12059: Filter out harmless warnings from Thrift compilation
This wraps Thrift compilation commands with the
bin/thrift-quiet-wrapper.sh shell script. This script runs the
Thrift compilation command while filtering out warnings
that we have no intention of fixing (e.g. warnings about
using 'binary' rather than 'list<byte>', etc).
This filters all warnings from hive_metastore.thrift, because
it is not under Impala control and generates a large number of
warnings.
When Thrift compilation fails, Thrift outputs messages with
"ERROR:" and "FAILURE:" and these are never filtered.
Testing:
- Ran "make gen-deps" on a clean build and verified that
the Thrift warnings are filtered
- Modified a Thrift file and verified errors are not
filtered
Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
---
A bin/thrift-quiet-wrapper.sh
M common/thrift/CMakeLists.txt
2 files changed, 56 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/19818/1
--
To view, visit http://gerrit.cloudera.org:8080/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
[Impala-ASF-CR] IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19818 )
Change subject: IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
......................................................................
Patch Set 5:
Thanks for the review
--
To view, visit http://gerrit.cloudera.org:8080/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 05 Jun 2023 22:39:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12059: Filter out harmless warnings from Thrift compilation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19818 )
Change subject: IMPALA-12059: Filter out harmless warnings from Thrift compilation
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
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-Comment-Date: Fri, 19 May 2023 02:31:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12059: Filter out harmless warnings from Thrift compilation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19818 )
Change subject: IMPALA-12059: Filter out harmless warnings from Thrift compilation
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12895/ : 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/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sat, 29 Apr 2023 00:37:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19818 )
Change subject: IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
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: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 02 Jun 2023 07:27:07 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19818 )
Change subject: IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9375/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 05 Jun 2023 22:38:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19818 )
Change subject: IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13176/ : 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/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
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: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 02 Jun 2023 01:19:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19818 )
Change subject: IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19818/3/bin/thrift-quiet-wrapper.sh
File bin/thrift-quiet-wrapper.sh:
http://gerrit.cloudera.org:8080/#/c/19818/3/bin/thrift-quiet-wrapper.sh@54
PS3, Line 54:
> Should we remove the temp file?
Oh, good point, I missed that.
--
To view, visit http://gerrit.cloudera.org:8080/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
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: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 05 Jun 2023 16:57:34 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12059: Filter out harmless warnings from Thrift compilation
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19818 )
Change subject: IMPALA-12059: Filter out harmless warnings from Thrift compilation
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19818/1/bin/thrift-quiet-wrapper.sh
File bin/thrift-quiet-wrapper.sh:
http://gerrit.cloudera.org:8080/#/c/19818/1/bin/thrift-quiet-wrapper.sh@33
PS1, Line 33: # 4. 'The "byte" type is a compatibility alias for "i8". Use "i8" to emphasize the signedness of this type.'
> line too long (108 > 90)
Done
--
To view, visit http://gerrit.cloudera.org:8080/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
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-Comment-Date: Thu, 18 May 2023 18:19:12 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19818
to look at the new patch set (#4).
Change subject: IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
......................................................................
IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
This wraps Thrift compilation commands with the
bin/thrift-quiet-wrapper.sh shell script. This script runs the
Thrift compilation command, collecting the output. If the
command fails, it dumps the output as-is. If the command succeeds,
it deduplicates the warnings and filters out a couple warnings
that we have no intention of fixing.
Testing:
- Ran "make gen-deps" on a clean build and verified that
the Thrift warnings are deduplicated and filtered
- Modified a Thrift file and verified errors are printed as-is.
Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
---
A bin/thrift-quiet-wrapper.sh
M common/thrift/CMakeLists.txt
2 files changed, 66 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/19818/4
--
To view, visit http://gerrit.cloudera.org:8080/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
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: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19818 )
Change subject: IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
LGTM. Just have a minor comment.
http://gerrit.cloudera.org:8080/#/c/19818/3/bin/thrift-quiet-wrapper.sh
File bin/thrift-quiet-wrapper.sh:
http://gerrit.cloudera.org:8080/#/c/19818/3/bin/thrift-quiet-wrapper.sh@54
PS3, Line 54:
Should we remove the temp file?
--
To view, visit http://gerrit.cloudera.org:8080/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
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: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 05 Jun 2023 13:19:29 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19818
to look at the new patch set (#3).
Change subject: IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
......................................................................
IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
This wraps Thrift compilation commands with the
bin/thrift-quiet-wrapper.sh shell script. This script runs the
Thrift compilation command, collecting the output. If the
command fails, it dumps the output as-is. If the command succeeds,
it deduplicates the warnings and filters out a couple warnings
that we have no intention of fixing.
Testing:
- Ran "make gen-deps" on a clean build and verified that
the Thrift warnings are deduplicated and filtered
- Modified a Thrift file and verified errors are printed as-is.
Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
---
A bin/thrift-quiet-wrapper.sh
M common/thrift/CMakeLists.txt
2 files changed, 65 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/19818/3
--
To view, visit http://gerrit.cloudera.org:8080/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
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: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19818 )
Change subject: IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9365/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
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: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 02 Jun 2023 02:02:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12059: Filter out harmless warnings from Thrift compilation
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19818
to look at the new patch set (#2).
Change subject: IMPALA-12059: Filter out harmless warnings from Thrift compilation
......................................................................
IMPALA-12059: Filter out harmless warnings from Thrift compilation
This wraps Thrift compilation commands with the
bin/thrift-quiet-wrapper.sh shell script. This script runs the
Thrift compilation command while filtering out warnings
that we have no intention of fixing (e.g. warnings about
using 'binary' rather than 'list<byte>', etc).
This filters all warnings from hive_metastore.thrift, because
it is not under Impala control and generates a large number of
warnings.
When Thrift compilation fails, Thrift outputs messages with
"ERROR:" and "FAILURE:" and these are never filtered.
Testing:
- Ran "make gen-deps" on a clean build and verified that
the Thrift warnings are filtered
- Modified a Thrift file and verified errors are not
filtered
Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
---
A bin/thrift-quiet-wrapper.sh
M common/thrift/CMakeLists.txt
2 files changed, 57 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/19818/2
--
To view, visit http://gerrit.cloudera.org:8080/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
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>
[Impala-ASF-CR] IMPALA-12059: Filter out harmless warnings from Thrift compilation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19818 )
Change subject: IMPALA-12059: Filter out harmless warnings from Thrift compilation
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9328/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
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-Comment-Date: Thu, 18 May 2023 21:10:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19818 )
Change subject: IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13198/ : 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/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
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: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 05 Jun 2023 17:20:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
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/19818 )
Change subject: IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
......................................................................
IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
This wraps Thrift compilation commands with the
bin/thrift-quiet-wrapper.sh shell script. This script runs the
Thrift compilation command, collecting the output. If the
command fails, it dumps the output as-is. If the command succeeds,
it deduplicates the warnings and filters out a couple warnings
that we have no intention of fixing.
Testing:
- Ran "make gen-deps" on a clean build and verified that
the Thrift warnings are deduplicated and filtered
- Modified a Thrift file and verified errors are printed as-is.
Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Reviewed-on: http://gerrit.cloudera.org:8080/19818
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
A bin/thrift-quiet-wrapper.sh
M common/thrift/CMakeLists.txt
2 files changed, 66 insertions(+), 9 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19818 )
Change subject: IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 06 Jun 2023 03:58:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19818 )
Change subject: IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
......................................................................
Patch Set 3:
Reworked this to deduplicate the warnings and only filter a couple less useful warnings.
Once we incorporate the fix for HIVE-27103 into our Hive dependency, the noise should be minimal. To prevent future issues for hive_metastore.thrift, I filed HIVE-27396 to use -strict for Thrift compilation.
--
To view, visit http://gerrit.cloudera.org:8080/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
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: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 02 Jun 2023 00:59:33 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12059: Filter out harmless warnings from Thrift compilation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19818 )
Change subject: IMPALA-12059: Filter out harmless warnings from Thrift compilation
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19818/1/bin/thrift-quiet-wrapper.sh
File bin/thrift-quiet-wrapper.sh:
http://gerrit.cloudera.org:8080/#/c/19818/1/bin/thrift-quiet-wrapper.sh@33
PS1, Line 33: # 4. 'The "byte" type is a compatibility alias for "i8". Use "i8" to emphasize the signedness of this type.'
line too long (108 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sat, 29 Apr 2023 00:17:30 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12059: Filter out harmless warnings from Thrift compilation
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19818 )
Change subject: IMPALA-12059: Filter out harmless warnings from Thrift compilation
......................................................................
Patch Set 2:
> Thanks for cleaning these up!
>
> Just have two questions:
> Are there compilation options in thrift compilers to ignore these
> warnings?
> Can we deduplicate the warnings instead of ignoring them?
There is an option to turn off all warnings, but I think we still care about warnings that are new. There is an option to disable the warning for 64-bit constants, and we could do that. There aren't equivalent flags for the other warnings we see. We could get stricter about new warnings from our thrift files by generating an error if there are unexpected warnings.
There is some deduplication that can be done within a single Thrift invocation, but it is not possible to deduplicate across the different Thrift invocations (which happen for each .thrift file). Part of the problem is that our CMake code is doing recursive generation, so some frequently used Thrift files generate warnings on each different invocation. That can be reworked, but it is a bigger change.
From my point of view, the warnings from our Thrift files have been there for years. I don't think we plan on fixing any of them, so I would rather filter them out than deduplicate them.
hive_metastore.thrift is a complicated case and I'm not sure what we should do. I think that is better addressed on the Hive side by failing Thrift generation if there are unexpected warnings (or adding some unit test that does the same check).
--
To view, visit http://gerrit.cloudera.org:8080/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
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: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 24 May 2023 20:06:19 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12059: Filter out harmless warnings from Thrift compilation
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19818 )
Change subject: IMPALA-12059: Filter out harmless warnings from Thrift compilation
......................................................................
Patch Set 2:
Thanks for cleaning these up!
Just have two questions:
Are there compilation options in thrift compilers to ignore these warnings?
Can we deduplicate the warnings instead of ignoring them?
--
To view, visit http://gerrit.cloudera.org:8080/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
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: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 19 May 2023 02:57:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12059: Filter out harmless warnings from Thrift compilation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19818 )
Change subject: IMPALA-12059: Filter out harmless warnings from Thrift compilation
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13077/ : 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/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
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-Comment-Date: Thu, 18 May 2023 18:40:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19818 )
Change subject: IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
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: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 05 Jun 2023 22:36:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19818 )
Change subject: IMPALA-12059: Deduplicate and filter warnings from Thrift compilation
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b912ac3d57d1a51e957889b5798dc05d156a3d0
Gerrit-Change-Number: 19818
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 05 Jun 2023 22:38:56 +0000
Gerrit-HasComments: No