Peter Rozsa has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22878 )

Change subject: IMPALA-14016: Add multi-catalog support for local catalog mode
......................................................................


Patch Set 11:

(4 comments)

Thanks, Zoltan! I also updated the logging file name template to better 
distinguish the separate startups for the REST Catalogs.

http://gerrit.cloudera.org:8080/#/c/22878/10/java/iceberg-rest-catalog-test/src/main/java/org/apache/iceberg/rest/IcebergRestCatalogTest.java
File 
java/iceberg-rest-catalog-test/src/main/java/org/apache/iceberg/rest/IcebergRestCatalogTest.java:

http://gerrit.cloudera.org:8080/#/c/22878/10/java/iceberg-rest-catalog-test/src/main/java/org/apache/iceberg/rest/IcebergRestCatalogTest.java@156
PS10, Line 156:
> nit: we usually don't put multiple statements onto a single line as it is s
Done


http://gerrit.cloudera.org:8080/#/c/22878/10/tests/common/iceberg_rest_server.py
File tests/common/iceberg_rest_server.py:

http://gerrit.cloudera.org:8080/#/c/22878/10/tests/common/iceberg_rest_server.py@61
PS10, Line 61:   def stop_rest_server(self, timeout_s=60):
             :     try:
             :       if self.process:
             :         os.killpg(self.pr
> I wonder if we should move it to a finally block, after the process is kill
Done


http://gerrit.cloudera.org:8080/#/c/22878/10/tests/custom_cluster/test_iceberg_rest_catalog.py
File tests/custom_cluster/test_iceberg_rest_catalog.py:

http://gerrit.cloudera.org:8080/#/c/22878/10/tests/custom_cluster/test_iceberg_rest_catalog.py@44
PS10, Line 44: def RestServerProperties(*server_configs):
> Btw, I like this decorator approach!
Thanks!


http://gerrit.cloudera.org:8080/#/c/22878/10/tests/custom_cluster/test_iceberg_rest_catalog.py@161
PS10, Line 161: start_args=NO_CATALOGD_STARTARGS)
> We don't have catalogd, so we probably don't need this. Same goes for L174.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbdd0f7085345e7954d9f6f264202699182dd1e1
Gerrit-Change-Number: 22878
Gerrit-PatchSet: 11
Gerrit-Owner: Peter Rozsa <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Peter Rozsa <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Fri, 19 Sep 2025 08:45:53 +0000
Gerrit-HasComments: Yes

Reply via email to