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