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

Reply via email to