Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22626 )
Change subject: IMPALA-13864: Implement ImpylaHS2ResultSet.exec_summary ...................................................................... Patch Set 4: (8 comments) http://gerrit.cloudera.org:8080/#/c/22626/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22626/3//COMMIT_MSG@15 PS3, Line 15: by > Nit: by Done http://gerrit.cloudera.org:8080/#/c/22626/3/shell/impala_client.py File shell/impala_client.py: http://gerrit.cloudera.org:8080/#/c/22626/3/shell/impala_client.py@141 PS3, Line 141: if not summary.nodes or len(summary.nodes) <= idx: > Would it make sense to also return if len(summary.nodes) <= idx? Not havin Make sense. Done. http://gerrit.cloudera.org:8080/#/c/22626/3/tests/beeswax/impala_beeswax.py File tests/beeswax/impala_beeswax.py: http://gerrit.cloudera.org:8080/#/c/22626/3/tests/beeswax/impala_beeswax.py@398 PS3, Line 398: return None > Can you please add a note about this change to the commit message? Done http://gerrit.cloudera.org:8080/#/c/22626/3/tests/common/impala_connection.py File tests/common/impala_connection.py: http://gerrit.cloudera.org:8080/#/c/22626/3/tests/common/impala_connection.py@113 PS3, Line 113: > Even though this import is only used here, please consider moving it to the Unfortunately, start-impala-cluster.py does not work if I do that. $ bin/start-impala-cluster.py Traceback (most recent call last): File "bin/start-impala-cluster.py", line 39, in <module> from tests.common.impala_cluster import (ImpalaCluster, DEFAULT_BEESWAX_PORT, File "/home/rsuminto/workspace/impala/tests/common/impala_cluster.py", line 37, in <module> from tests.common.impala_service import ( File "/home/rsuminto/workspace/impala/tests/common/impala_service.py", line 34, in <module> from tests.common.impala_connection import create_connection, create_ldap_connection File "/home/rsuminto/workspace/impala/tests/common/impala_connection.py", line 36, in <module> from shell.impala_client import build_exec_summary_table File "/home/rsuminto/workspace/impala/shell/impala_client.py", line 21, in <module> from compatibility import _xrange as xrange ModuleNotFoundError: No module named 'compatibility' http://gerrit.cloudera.org:8080/#/c/22626/3/tests/common/impala_connection.py@305 PS3, Line 305: > Nit: passed Done http://gerrit.cloudera.org:8080/#/c/22626/3/tests/common/impala_connection.py@487 PS3, Line 487: t, user > Nit: supports Done http://gerrit.cloudera.org:8080/#/c/22626/3/tests/custom_cluster/test_executor_groups.py File tests/custom_cluster/test_executor_groups.py: http://gerrit.cloudera.org:8080/#/c/22626/3/tests/custom_cluster/test_executor_groups.py@a1053 PS3, Line 1053: > Does self.hs2_client get re-initialized before this test begins executing? Yes. Custom cluster test will recreate clients on each for each test method, because it restart Impala cluster. https://github.com/apache/impala/blob/master/tests/common/custom_cluster_test_suite.py#L365 http://gerrit.cloudera.org:8080/#/c/22626/3/tests/custom_cluster/test_executor_groups.py@992 PS3, Line 992: self.hs2_client.set_configuration_option(k, v) > Please note this change in the commit message. Done -- To view, visit http://gerrit.cloudera.org:8080/22626 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7d88f78e58eeda29ce21e7828884c7a129d7efe6 Gerrit-Change-Number: 22626 Gerrit-PatchSet: 4 Gerrit-Owner: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Comment-Date: Wed, 19 Mar 2025 22:27:07 +0000 Gerrit-HasComments: Yes