Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21443 )

Change subject: IMPALA-12869: Shorten argument list to enable local catalog mode
......................................................................


Patch Set 17:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/21443/17//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21443/17//COMMIT_MSG@10
PS17, Line 10: bin/start-impala-cluster.py 
--catalogd_args=--catalog_topic_mode=minimal --impalad_args=--use_local_catalog
nit: Break this long line to not exceed 72 chars:

bin/start-impala-cluster.py \
  --catalogd_args=--catalog_topic_mode=minimal \
  --impalad_args=--use_local_catalog


http://gerrit.cloudera.org:8080/#/c/21443/15/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/21443/15/bin/start-impala-cluster.py@204
PS15, Line 204: --use_on_demand_met
> It is also possible to have combination of  --catalogd_args=--catalog_topic
The last part of this comment has not been addressed. I will clarify my request 
in patch set 17.


http://gerrit.cloudera.org:8080/#/c/21443/17/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/21443/17/bin/start-impala-cluster.py@204
PS17, Line 204: parser.add_option("--use_on_demand_metadata", 
dest="use_on_demand_metadata"
Please edit existing test to prove that this new options work. These are some 
good candidate:
custom_cluster/test_concurrent_ddls.py::TestConcurrentDdls::test_local_catalog_ddls_with_invalidate_metadata
custom_cluster/test_concurrent_ddls.py::TestConcurrentDdls::test_local_catalog_ddls_with_invalidate_metadata_sync_ddl


http://gerrit.cloudera.org:8080/#/c/21443/17/bin/start-impala-cluster.py@212
PS17, Line 212: "
> flake8: E501 line too long (92 > 90 characters)
I agree with Michael comment that it is not worth to have dedicated option 
'use_on_demand_metadata_mixed'.
https://gerrit.cloudera.org/c/21443/15/bin/start-impala-cluster.py#204

Please remove this.


http://gerrit.cloudera.org:8080/#/c/21443/17/bin/start-impala-cluster.py@469
PS17, Line 469:   return catalogd_arg_list
Before returning, assert that there is only 1 'catalog_topic_mode' arg in 
catalogd_arg_list.


http://gerrit.cloudera.org:8080/#/c/21443/17/bin/start-impala-cluster.py@694
PS17, Line 694:   return impalad_args
Before returning, assert that there is only 1 'use_local_catalog' arg in 
impalad_args.


http://gerrit.cloudera.org:8080/#/c/21443/17/bin/start-impala-cluster.py@1175
PS17, Line 1175:   LOG.info("Starting Impala cluster with the following 
configuration:")
               :   LOG.info("Cluster size: %d", options.cluster_size)
Remove this. This is redundant with LOG in L1169.



--
To view, visit http://gerrit.cloudera.org:8080/21443
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iab5f3613b39a8473ba1f6ab7cb2634d87b808142
Gerrit-Change-Number: 21443
Gerrit-PatchSet: 17
Gerrit-Owner: Anshula Jain <[email protected]>
Gerrit-Reviewer: Anshula Jain <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Comment-Date: Mon, 24 Feb 2025 19:59:27 +0000
Gerrit-HasComments: Yes

Reply via email to