Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20344 )
Change subject: IMPALA-10120: Add required fields for TGetInfoResp when error. ...................................................................... Patch Set 5: (6 comments) > Patch Set 5: > > (1 comment) > > Hi quanlong, I've add the ee test, but I can't figure out where jenkins job > failed:( The failure is a network issue and unrelated to this change. We can ignore it. https://jenkins.impala.io/job/all-build-options-ub2004/206/console subprocess.CalledProcessError: Command '['wget', '-q', 'https://www.apache.org/dyn/closer.cgi/hive/hive-3.1.3/apache-hive-3.1.3-bin.tar.gz?action=download', '--output-document=/home/ubuntu/Impala/toolchain/apache_components/apache-hive-3.1.3-bin.tar.gz']' returned non-zero exit status 8 http://gerrit.cloudera.org:8080/#/c/20344/2/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/20344/2/be/src/service/impala-hs2-server.cc@479 PS2, Line 479: RETURN_IF_ERROR(return_val, THandleIdentifi > I think no need. It seems that only TGetInfoResp has this problem, and info Please add a comment here: 'infoValue' is a required field of TGetInfoResp http://gerrit.cloudera.org:8080/#/c/20344/5/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/20344/5/be/src/service/impala-hs2-server.cc@1011 PS5, Line 1011: return_val It seems TGetLogResp could have the same issue in error handling. Its 'log' field is also required: struct TGetLogResp { 1: required TStatus status 2: required string log } We can use a specifit macro for TGetLogResp, just like HS2_RETURN_IF_ERROR but set the 'log' field in it. http://gerrit.cloudera.org:8080/#/c/20344/5/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: http://gerrit.cloudera.org:8080/#/c/20344/5/tests/common/impala_test_suite.py@1019 PS5, Line 1019: run_stmt_in_impala nit: the method name doesn't indicate it's using beeline, what about 'run_impala_stmt_in_beeline'? http://gerrit.cloudera.org:8080/#/c/20344/5/tests/shell/test_beeline.py File tests/shell/test_beeline.py: http://gerrit.cloudera.org:8080/#/c/20344/5/tests/shell/test_beeline.py@21 PS5, Line 21: from __future__ import absolute_import, division, print_function unused imports? http://gerrit.cloudera.org:8080/#/c/20344/5/tests/shell/test_beeline.py@27 PS5, Line 27: def test_beeline_compatibility(self): Let's create the new table in 'unique_database'. We just need a new parameter 'unique_database' here. The test framework will assign a unique database name and create, drop this database. http://gerrit.cloudera.org:8080/#/c/20344/5/tests/shell/test_beeline.py@29 PS5, Line 29: self.run_stmt_in_impala(stmt, default_db='functional') Can we verify the results? -- To view, visit http://gerrit.cloudera.org:8080/20344 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib42bb82735fb4a8e6911b6a19adb8bd84973300b Gerrit-Change-Number: 20344 Gerrit-PatchSet: 5 Gerrit-Owner: Xiang Yang <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Xiang Yang <[email protected]> Gerrit-Comment-Date: Sat, 07 Oct 2023 02:50:50 +0000 Gerrit-HasComments: Yes
