Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22353 )

Change subject: IMPALA-13586: Initial support for Iceberg REST Catalogs
......................................................................


Patch Set 2:

(18 comments)

gerrit-auto-critic failed. You can reproduce it locally using command:

  python3 bin/jenkins/critique-gerrit-review.py --dryrun

To run it, you might need a virtual env with Python3's venv installed.

http://gerrit.cloudera.org:8080/#/c/22353/2/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:

http://gerrit.cloudera.org:8080/#/c/22353/2/be/src/common/global-flags.cc@437
PS2, Line 437: DEFINE_string(iceberg_rest_warehouse_location, "", "Iceberg REST 
catalog warehouse location.");
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/22353/2/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergRESTCatalog.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergRESTCatalog.java:

http://gerrit.cloudera.org:8080/#/c/22353/2/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergRESTCatalog.java@116
PS2, Line 116:                          Map<String, String> properties) throws 
IcebergTableLoadingException {
line too long (94 > 90)


http://gerrit.cloudera.org:8080/#/c/22353/2/fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java:

http://gerrit.cloudera.org:8080/#/c/22353/2/fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java@103
PS2, Line 103: import static 
org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READONLY;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/22353/2/fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java@489
PS2, Line 489:                                               String partName, 
Partition msPartition, ListMap<TNetworkAddress> hostIndex)
line too long (121 > 90)


http://gerrit.cloudera.org:8080/#/c/22353/2/fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java@541
PS2, Line 541:                                  ImmutableList<FileDescriptor> 
fds, ImmutableList<FileDescriptor> insertFds,
line too long (108 > 90)


http://gerrit.cloudera.org:8080/#/c/22353/2/fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java@634
PS2, Line 634:           msTable.getPartitionKeysSize(), 
Lists.newArrayList(msTable.getSd().getLocation()));
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/22353/2/fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java@691
PS2, Line 691:                                                   TableMetaRef 
table, Map<PartitionRef, PartitionMetadata> metas) throws TException {
line too long (133 > 90)


http://gerrit.cloudera.org:8080/#/c/22353/2/fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java@718
PS2, Line 718:                                                       
TableParams params, Table msTable) throws TException {
line too long (108 > 90)


http://gerrit.cloudera.org:8080/#/c/22353/2/fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java@727
PS2, Line 727:       org.apache.iceberg.Table apiTable, 
ListMap<TNetworkAddress> hostIndex) throws TException {
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/22353/2/fe/src/main/java/org/apache/impala/util/IcebergUtil.java
File fe/src/main/java/org/apache/impala/util/IcebergUtil.java:

http://gerrit.cloudera.org:8080/#/c/22353/2/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@132
PS2, Line 132:   public static final String ICEBERG_REST_WAREHOUSE_LOCATION = 
"iceberg_rest_warehouse_location";
line too long (97 > 90)


http://gerrit.cloudera.org:8080/#/c/22353/2/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@1070
PS2, Line 1070:   private static int 
createIcebergMetadata(org.apache.iceberg.Table iceTbl, FlatBufferBuilder fbb,
line too long (98 > 90)


http://gerrit.cloudera.org:8080/#/c/22353/2/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@1115
PS2, Line 1115:   private static int 
createPartitionKeys(org.apache.iceberg.Table iceTbl, FlatBufferBuilder fbb,
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/22353/2/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@1236
PS2, Line 1236:         ICEBERG_REST_URI, ICEBERG_REST_USER_ID, 
ICEBERG_REST_USER_SECRET, ICEBERG_REST_WAREHOUSE_LOCATION));
line too long (108 > 90)


http://gerrit.cloudera.org:8080/#/c/22353/2/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/22353/2/java/iceberg-rest-catalog-test/src/main/java/org/apache/iceberg/rest/IcebergRestCatalogTest.java@54
PS2, Line 54:   static final String WAREHOUSE_LOCATION = 
"hdfs://localhost:20500/test-warehouse/iceberg_test/hadoop_catalog";
line too long (111 > 90)


http://gerrit.cloudera.org:8080/#/c/22353/2/java/iceberg-rest-catalog-test/src/main/java/org/apache/iceberg/rest/IcebergRestCatalogTest.java@121
PS2, Line 121:          // use Map so that Jackson doesn't try to instantiate 
ImmutableMap from payload.getClass()
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/22353/2/java/iceberg-rest-catalog-test/src/main/java/org/apache/iceberg/rest/IcebergRestCatalogTest.java@128
PS2, Line 128:            String.format("Failed to serialize and deserialize 
%s: %s", description, payload), e);
line too long (97 > 90)


http://gerrit.cloudera.org:8080/#/c/22353/2/tests/custom_cluster/test_iceberg_rest_catalog.py
File tests/custom_cluster/test_iceberg_rest_catalog.py:

http://gerrit.cloudera.org:8080/#/c/22353/2/tests/custom_cluster/test_iceberg_rest_catalog.py@23
PS2, Line 23: import sys
flake8: F401 'sys' imported but unused


http://gerrit.cloudera.org:8080/#/c/22353/2/tests/custom_cluster/test_iceberg_rest_catalog.py@28
PS2, Line 28: "
flake8: E225 missing whitespace around operator



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1722b898b568d2f5689002f2b9bef59320cb088c
Gerrit-Change-Number: 22353
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <borokna...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Comment-Date: Mon, 27 Jan 2025 17:15:04 +0000
Gerrit-HasComments: Yes

Reply via email to