Re: [I] [Improvement] The Gravitino CLI shoudl show help if no arguments are passed [gravitino]
justinmclean commented on issue #5807: URL: https://github.com/apache/gravitino/issues/5807#issuecomment-2533468972 I'd go with the first one -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Bug report] Gravitino trino-connector drop catalog will cause Trino Coordinator OOM [gravitino]
diqiu50 commented on issue #5804: URL: https://github.com/apache/gravitino/issues/5804#issuecomment-2533470563 This issue is caused by the resource release triggered by the Trino `DROP CATALOG` command. For more information, refer tohttps://trino.io/docs/current/admin/properties-catalog.html Frequent modifications to the catalog will cause this issue. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Bug report] Gravitino trino-connector drop catalog will cause Trino Coordinator OOM [gravitino]
maomaodev commented on issue #5804: URL: https://github.com/apache/gravitino/issues/5804#issuecomment-2533471056 您好,我是毛立夫。 您的邮件已收到。 祝您生活愉快,身体健康! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Improvement] The Gravitino CLI shoudl show help if no arguments are passed [gravitino]
Abyss-lord commented on issue #5807: URL: https://github.com/apache/gravitino/issues/5807#issuecomment-2533480986 > I'd go with the first one Thank you Justin, I have submitted a PR,Could you please take a look when you have time? #5821 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] [#5807] improvement(CLI): Add functionality to display help information when no args are passed [gravitino]
Abyss-lord opened a new pull request, #5821: URL: https://github.com/apache/gravitino/pull/5821 ### What changes were proposed in this pull request? Add functionality to display help information when no arguments are passed. ### Why are the changes needed? Fix: #5807 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? bash test https://github.com/user-attachments/assets/86945a42-9d17-4fb2-a515-731a1299c9e2";> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5734] feat (gvfs-fuse): Gvfs-fuse basic FUSE-level implementation and code structure layout [gravitino]
mchades commented on code in PR #5738: URL: https://github.com/apache/gravitino/pull/5738#discussion_r1879218669 ## .github/workflows/gvfs-fuse-build-test.yml: ## @@ -0,0 +1,93 @@ +name: Build gvfs-fuse and testing + +# Controls when the workflow will run +on: + push: +branches: [ "main", "branch-*" ] + pull_request: +branches: [ "main", "branch-*" ] + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + changes: +runs-on: ubuntu-latest +steps: + - uses: actions/checkout@v3 + - uses: dorny/paths-filter@v2 +id: filter +with: + filters: | +source_changes: + - .github/** + - api/** + - bin/** + - catalogs/** + - clients/filesystem-fuse/** + - common/** + - conf/** + - core/** + - dev/** + - gradle/** + - meta/** + - scripts/** + - server/** + - server-common/** + - build.gradle.kts + - gradle.properties + - gradlew + - setting.gradle.kts +outputs: + source_changes: ${{ steps.filter.outputs.source_changes }} + + # Build for AMD64 architecture + Gvfs-Build: +needs: changes +if: needs.changes.outputs.source_changes == 'true' +runs-on: ubuntu-latest +timeout-minutes: 60 +strategy: + matrix: +architecture: [linux/amd64] +java-version: [ 17 ] +env: + PLATFORM: ${{ matrix.architecture }} +steps: + - uses: actions/checkout@v3 + + - uses: actions/setup-java@v4 +with: + java-version: ${{ matrix.java-version }} + distribution: 'temurin' + cache: 'gradle' + + - name: Set up QEMU +uses: docker/setup-qemu-action@v2 + + - name: Check required command +run: | + dev/ci/check_commands.sh + + - name: Build and test Gravitino +run: | + ./gradlew :clients:filesystem-fuse:build -PenableFuse=true + + - name: Package Gravitino +run: | + ./gradlew compileDistribution -x test -PjdkVersion=${{ matrix.java-version }} -PenableFuse=true Review Comment: what's the usage of `-PenableFuse=true` here -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5623] feat(python): supports credential API in python client [gravitino]
FANNG1 commented on PR #5777: URL: https://github.com/apache/gravitino/pull/5777#issuecomment-2533469002 @jerryshao @xloya , all comments are addressed, please help to review again -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Subtask] Add e2e test case for creating schema [gravitino]
xunliu closed issue #5796: [Subtask] Add e2e test case for creating schema URL: https://github.com/apache/gravitino/issues/5796 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Subtask] Add e2e test case for creating schema [gravitino]
xunliu closed issue #5796: [Subtask] Add e2e test case for creating schema URL: https://github.com/apache/gravitino/issues/5796 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5796][#5798] Add e2e test case for creating schema/fileset/topic/table [gravitino]
xunliu merged PR #5805: URL: https://github.com/apache/gravitino/pull/5805 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
(gravitino) branch main updated: [#5796][#5798] Add e2e test case for creating schema/fileset/topic/table (#5805)
This is an automated email from the ASF dual-hosted git repository. liuxun pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/gravitino.git The following commit(s) were added to refs/heads/main by this push: new 692dfb409 [#5796][#5798] Add e2e test case for creating schema/fileset/topic/table (#5805) 692dfb409 is described below commit 692dfb409368d43452ff0c46a8d83f9dab767d6e Author: Qian Xia AuthorDate: Wed Dec 11 10:42:39 2024 +0800 [#5796][#5798] Add e2e test case for creating schema/fileset/topic/table (#5805) ### What changes were proposed in this pull request? Add e2e test case for creating schema/fileset/topic/table ### Why are the changes needed? we need to implement the created test case by simulating a click on the UI dialog box Fix: #5796, #5797, #5798, #5799 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? local run e2e test https://github.com/user-attachments/assets/de5e716b-0956-46ed-a41d-3d619dacfb3f";> https://github.com/user-attachments/assets/c69b560a-6cab-424f-92b1-1dba46a447de";> https://github.com/user-attachments/assets/4ff54f73-468b-4ea7-b413-d6e745893492";> --- .../test/web/ui/CatalogsPageKafkaTest.java | 50 + .../integration/test/web/ui/CatalogsPageTest.java | 117 +-- .../test/web/ui/pages/CatalogsPage.java| 224 - .../metalake/rightContent/CreateTableDialog.js | 1 + .../metalake/rightContent/RightContent.js | 2 +- 5 files changed, 288 insertions(+), 106 deletions(-) diff --git a/web/integration-test/src/test/java/org/apache/gravitino/integration/test/web/ui/CatalogsPageKafkaTest.java b/web/integration-test/src/test/java/org/apache/gravitino/integration/test/web/ui/CatalogsPageKafkaTest.java index 8af7277e2..61278114e 100644 --- a/web/integration-test/src/test/java/org/apache/gravitino/integration/test/web/ui/CatalogsPageKafkaTest.java +++ b/web/integration-test/src/test/java/org/apache/gravitino/integration/test/web/ui/CatalogsPageKafkaTest.java @@ -20,12 +20,8 @@ package org.apache.gravitino.integration.test.web.ui; import java.util.Arrays; -import java.util.Collections; import java.util.List; -import org.apache.gravitino.Catalog; -import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.client.GravitinoAdminClient; -import org.apache.gravitino.client.GravitinoMetalake; import org.apache.gravitino.integration.test.container.ContainerSuite; import org.apache.gravitino.integration.test.web.ui.pages.CatalogsPage; import org.apache.gravitino.integration.test.web.ui.pages.MetalakePage; @@ -46,7 +42,6 @@ public class CatalogsPageKafkaTest extends BaseWebIT { private static final ContainerSuite containerSuite = ContainerSuite.getInstance(); protected static GravitinoAdminClient gravitinoClient; - private static GravitinoMetalake metalake; protected static String gravitinoUri = "http://127.0.0.1:8090";; protected static String kafkaUri = "http://127.0.0.1:9092";; @@ -76,35 +71,6 @@ public class CatalogsPageKafkaTest extends BaseWebIT { catalogsPage = new CatalogsPage(driver); } - /** - * Creates a Kafka topic within the specified Metalake, Catalog, Schema, and Topic names. - * - * @param metalakeName The name of the Metalake. - * @param catalogName The name of the Catalog. - * @param schemaName The name of the Schema. - * @param topicName The name of the Kafka topic. - */ - void createTopic(String metalakeName, String catalogName, String schemaName, String topicName) { -Catalog catalog_kafka = metalake.loadCatalog(catalogName); -catalog_kafka -.asTopicCatalog() -.createTopic( -NameIdentifier.of(schemaName, topicName), "comment", null, Collections.emptyMap()); - } - - /** - * Drops a Kafka topic from the specified Metalake, Catalog, and Schema. - * - * @param metalakeName The name of the Metalake where the topic resides. - * @param catalogName The name of the Catalog that contains the topic. - * @param schemaName The name of the Schema under which the topic exists. - * @param topicName The name of the Kafka topic to be dropped. - */ - void dropTopic(String metalakeName, String catalogName, String schemaName, String topicName) { -Catalog catalog_kafka = metalake.loadCatalog(catalogName); -catalog_kafka.asTopicCatalog().dropTopic(NameIdentifier.of(schemaName, topicName)); - } - @Test @Order(0) public void testCreateKafkaCatalog() throws InterruptedException { @@ -113,7 +79,7 @@ public class CatalogsPageKafkaTest extends BaseWebIT { metalakePage.setMetalakeNameField(METALAKE_NAME); clickAndWait(metalakePage.submitHandleMetalakeBtn); // load metalake -metalake = gravitinoClient.loadMetalake(METALAKE_NAME); +gravitinoClient.loadMetalake(METALAKE_NAME); metalakePage.clickMetalakeLink(
Re: [PR] [#5778] feat(aliyun-bundles)support OSS secret key credential [gravitino]
FANNG1 commented on code in PR #5814: URL: https://github.com/apache/gravitino/pull/5814#discussion_r1879221330 ## api/src/main/java/org/apache/gravitino/credential/OSSSecretKeyCredential.java: ## @@ -0,0 +1,116 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.credential; + +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableMap; +import java.util.Map; +import org.apache.commons.lang3.StringUtils; + +/** OSS secret key credential. */ +public class OSSSecretKeyCredential implements Credential { + + /** OSS secret key credential type. */ + public static final String OSS_SECRET_KEY_CREDENTIAL_TYPE = "oss-secret-key"; + /** The static access key ID used to access OSS data. */ + public static final String GRAVITINO_OSS_STATIC_ACCESS_KEY_ID = "oss-access-key-id"; + /** The static secret access key used to access OSS data. */ + public static final String GRAVITINO_OSS_STATIC_SECRET_ACCESS_KEY = "oss-secret-access-key"; + + private String accessKeyId; + private String secretAccessKey; + + /** + * Constructs an instance of {@link OSSSecretKeyCredential} with the static OSS access key ID and + * secret access key. + * + * @param accessKeyId The OSS static access key ID. + * @param secretAccessKey The OSS static secret access key. + */ + public OSSSecretKeyCredential(String accessKeyId, String secretAccessKey) { +Preconditions.checkNotNull(accessKeyId, "OSS access key Id should not null"); Review Comment: could you reuse `validate` here? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#3888] Metadata Support ClickHouse [gravitino]
flaming-archer commented on PR #5819: URL: https://github.com/apache/gravitino/pull/5819#issuecomment-2533506440 > Can we split this to a few smaller PRs? One PR with 5000+ lines changes in 38 files is too big to review. Among the 5000 lines, most of them are ck code, and most of them are ck test code. As a new catlog, it may still be quite normal. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#3888] Metadata Support ClickHouse [gravitino]
flaming-archer commented on code in PR #5819: URL: https://github.com/apache/gravitino/pull/5819#discussion_r1879251194 ## catalogs/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/ClickHouseTypeConverter.java: ## @@ -0,0 +1,193 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.catalog.clickhouse.converter; + +import org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter; +import org.apache.gravitino.rel.types.Type; +import org.apache.gravitino.rel.types.Types; + +/** Type converter for ClickHouse. */ +public class ClickHouseTypeConverter extends JdbcTypeConverter { + + static final String INT8 = "Int8"; + static final String INT16 = "Int16"; + static final String INT32 = "Int32"; + static final String INT64 = "Int64"; + static final String INT128 = "Int128"; + static final String INT256 = "Int256"; + static final String UINT8 = "UInt8"; + static final String UINT16 = "UInt16"; + static final String UINT32 = "UInt32"; + static final String UINT64 = "UInt64"; + static final String UINT128 = "UInt128"; + static final String UINT256 = "UInt256"; + + static final String FLOAT32 = "Float32"; + static final String FLOAT64 = "Float64"; + static final String BFLOAT16 = "BFloat16"; + static final String DECIMAL = "Decimal"; + static final String STRING = "String"; + static final String FIXEDSTRING = "FixedString"; + static final String DATE = "Date"; + static final String DATE32 = "Date32"; + static final String DATETIME = "DateTime"; + static final String DATETIME64 = "DateTime64"; + static final String ENUM = "Enum"; + static final String BOOL = "Bool"; + static final String UUID = "UUID"; + + // bellow is Object Data Type + static final String IPV4 = "IPv4"; + static final String IPV6 = "IPv6"; + static final String ARRAY = "Array"; + static final String TUPLE = "Tuple"; + static final String MAP = "Map"; + static final String VARIANT = "Variant"; + static final String LOWCARDINALITY = "LowCardinality"; + static final String NULLABLE = "Nullable"; + static final String AGGREGATEFUNCTION = "AggregateFunction"; + static final String SIMPLEAGGREGATEFUNCTION = "SimpleAggregateFunction"; + static final String GEO = "Geo"; + + // bellow is Special Data Types + static final String Domains = "Domains"; + static final String Nested = "Nested"; + static final String Dynamic = "Dynamic"; + static final String JSON = "JSON"; + + @Override + public Type toGravitino(JdbcTypeBean typeBean) { +String typeName = typeBean.getTypeName(); +if (typeName.startsWith("Nullable(")) { + typeName = typeName.substring(9, typeName.length() - 1); +} + +if (typeName.startsWith("Decimal(")) { + typeName = "Decimal"; +} + +if (typeName.startsWith("FixedString(")) { + typeName = "FixedString"; +} + +switch (typeName) { + case INT8: +return Types.ByteType.get(); + case INT16: +return Types.ShortType.get(); + case INT32: +return Types.IntegerType.get(); + case INT64: +return Types.LongType.get(); + case UINT8: +return Types.ByteType.unsigned(); + case UINT16: +return Types.ShortType.unsigned(); + case UINT32: +return Types.IntegerType.unsigned(); + case UINT64: +return Types.LongType.unsigned(); + case FLOAT32: +return Types.FloatType.get(); + case FLOAT64: +return Types.DoubleType.get(); + case DECIMAL: +return Types.DecimalType.of(typeBean.getColumnSize(), typeBean.getScale()); + case STRING: +return Types.StringType.get(); + case FIXEDSTRING: +return Types.FixedCharType.of(typeBean.getColumnSize()); + case DATE: +return Types.DateType.get(); + case DATE32: +return Types.DateType.get(); + case DATETIME: +return Types.TimestampType.withoutTimeZone(); + case DATETIME64: +return Types.TimestampType.withoutTimeZone(); + case BOOL: +return Types.BooleanType.get(); + case UUID: +return Types.UUIDType.get(); + default: +return Types.ExternalType.of(t
Re: [PR] [#3888] Metadata Support ClickHouse [gravitino]
flaming-archer commented on code in PR #5819: URL: https://github.com/apache/gravitino/pull/5819#discussion_r1879253856 ## catalogs/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/ClickHouseTypeConverter.java: ## @@ -0,0 +1,193 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.catalog.clickhouse.converter; + +import org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter; +import org.apache.gravitino.rel.types.Type; +import org.apache.gravitino.rel.types.Types; + +/** Type converter for ClickHouse. */ +public class ClickHouseTypeConverter extends JdbcTypeConverter { + + static final String INT8 = "Int8"; + static final String INT16 = "Int16"; + static final String INT32 = "Int32"; + static final String INT64 = "Int64"; + static final String INT128 = "Int128"; + static final String INT256 = "Int256"; + static final String UINT8 = "UInt8"; + static final String UINT16 = "UInt16"; + static final String UINT32 = "UInt32"; + static final String UINT64 = "UInt64"; + static final String UINT128 = "UInt128"; + static final String UINT256 = "UInt256"; + + static final String FLOAT32 = "Float32"; + static final String FLOAT64 = "Float64"; + static final String BFLOAT16 = "BFloat16"; + static final String DECIMAL = "Decimal"; + static final String STRING = "String"; + static final String FIXEDSTRING = "FixedString"; + static final String DATE = "Date"; + static final String DATE32 = "Date32"; + static final String DATETIME = "DateTime"; + static final String DATETIME64 = "DateTime64"; + static final String ENUM = "Enum"; + static final String BOOL = "Bool"; + static final String UUID = "UUID"; + + // bellow is Object Data Type + static final String IPV4 = "IPv4"; + static final String IPV6 = "IPv6"; + static final String ARRAY = "Array"; + static final String TUPLE = "Tuple"; + static final String MAP = "Map"; + static final String VARIANT = "Variant"; + static final String LOWCARDINALITY = "LowCardinality"; + static final String NULLABLE = "Nullable"; + static final String AGGREGATEFUNCTION = "AggregateFunction"; + static final String SIMPLEAGGREGATEFUNCTION = "SimpleAggregateFunction"; + static final String GEO = "Geo"; + + // bellow is Special Data Types + static final String Domains = "Domains"; + static final String Nested = "Nested"; + static final String Dynamic = "Dynamic"; + static final String JSON = "JSON"; + + @Override + public Type toGravitino(JdbcTypeBean typeBean) { +String typeName = typeBean.getTypeName(); +if (typeName.startsWith("Nullable(")) { + typeName = typeName.substring(9, typeName.length() - 1); +} + +if (typeName.startsWith("Decimal(")) { + typeName = "Decimal"; +} + +if (typeName.startsWith("FixedString(")) { + typeName = "FixedString"; +} + +switch (typeName) { + case INT8: +return Types.ByteType.get(); + case INT16: +return Types.ShortType.get(); + case INT32: +return Types.IntegerType.get(); + case INT64: +return Types.LongType.get(); + case UINT8: +return Types.ByteType.unsigned(); + case UINT16: +return Types.ShortType.unsigned(); + case UINT32: +return Types.IntegerType.unsigned(); + case UINT64: +return Types.LongType.unsigned(); + case FLOAT32: +return Types.FloatType.get(); + case FLOAT64: +return Types.DoubleType.get(); + case DECIMAL: +return Types.DecimalType.of(typeBean.getColumnSize(), typeBean.getScale()); + case STRING: +return Types.StringType.get(); + case FIXEDSTRING: +return Types.FixedCharType.of(typeBean.getColumnSize()); + case DATE: +return Types.DateType.get(); + case DATE32: +return Types.DateType.get(); + case DATETIME: +return Types.TimestampType.withoutTimeZone(); + case DATETIME64: +return Types.TimestampType.withoutTimeZone(); + case BOOL: +return Types.BooleanType.get(); + case UUID: +return Types.UUIDType.get(); + default: +return Types.ExternalType.of(t
Re: [PR] [#5602] feat(core): Add model storage schema layout (part-2) [gravitino]
jerryshao commented on code in PR #5728: URL: https://github.com/apache/gravitino/pull/5728#discussion_r1879260947 ## core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/ModelMetaBaseSQLProvider.java: ## @@ -134,4 +134,11 @@ public String deleteModelMetasByLegacyTimeline( + ModelMetaMapper.TABLE_NAME + " WHERE deleted_at > 0 AND deleted_at < #{legacyTimeline} LIMIT #{limit}"; } + + public String updateModelLatestVersion(@Param("modelId") Long modelId) { +return "UPDATE " ++ ModelMetaMapper.TABLE_NAME ++ " SET model_latest_version = model_latest_version + 1" ++ " WHERE model_id = #{modelId} AND deleted_at = 0"; Review Comment: I think typically it will not have the concurrent updates problem for a single server Gravitino, looks like it only happens when we have multiple Gravitino instances. I think using `update...set...where` may not really solve the problem, a possible solution is to use transactions to handle this problem. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5624] feat(bundles): support ADLS credential provider [gravitino]
FANNG1 commented on code in PR #5737: URL: https://github.com/apache/gravitino/pull/5737#discussion_r1879283913 ## common/src/main/java/org/apache/gravitino/credential/CredentialPropertyUtils.java: ## @@ -53,7 +54,9 @@ public class CredentialPropertyUtils { OSSTokenCredential.GRAVITINO_OSS_SESSION_ACCESS_KEY_ID, ICEBERG_OSS_ACCESS_KEY_ID, OSSTokenCredential.GRAVITINO_OSS_SESSION_SECRET_ACCESS_KEY, - ICEBERG_OSS_ACCESS_KEY_SECRET); + ICEBERG_OSS_ACCESS_KEY_SECRET, + ADLSTokenCredential.GRAVITINO_ADLS_SAS_TOKEN, + ICEBERG_ADLS_TOKEN); Review Comment: The iceberg token name is not correct, it should be `adls.sas-token.{account_name}` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#3888] Metadata Support ClickHouse [gravitino]
mchades commented on PR #5819: URL: https://github.com/apache/gravitino/pull/5819#issuecomment-2533563155 Hi @flaming-archer thanks a lot for your contribution. Since this is a big feature, would you please create a epic issue and break down things into small tasks, also it would be nice to have a design doc beforehand. You can check and follow what OceanBase did (https://github.com/apache/gravitino/issues/4848). Thanks a lot. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Improvement] When a malformed name is passed to the CLI command to list columns in a table an exception occurs [gravitino]
justinmclean commented on issue #5808: URL: https://github.com/apache/gravitino/issues/5808#issuecomment-2533565969 Done. Feel free to add a comment saying you are working on an issue without me assigning it to you. I can assign it to you after you have started working on it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#3888] Metadata Support ClickHouse [gravitino]
jerryshao commented on PR #5819: URL: https://github.com/apache/gravitino/pull/5819#issuecomment-2533566188 Hi @flaming-archer thanks a lot for your contribution, can we just break down into small subtasks, and start with a design doc? @mchades from the community will help you to get everything merged. Thanks a lot. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5624] feat(bundles): support ADLS credential provider [gravitino]
FANNG1 commented on code in PR #5737: URL: https://github.com/apache/gravitino/pull/5737#discussion_r1879286069 ## iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java: ## @@ -78,7 +78,9 @@ public class IcebergCatalogWrapper implements AutoCloseable { IcebergConstants.ICEBERG_S3_ENDPOINT, IcebergConstants.ICEBERG_OSS_ENDPOINT, IcebergConstants.ICEBERG_OSS_ACCESS_KEY_ID, - IcebergConstants.ICEBERG_OSS_ACCESS_KEY_SECRET); + IcebergConstants.ICEBERG_OSS_ACCESS_KEY_SECRET, Review Comment: please remove `ICEBERG_ADLS_STORAGE_ACCOUNT_NAME ` and `ICEBERG_ADLS_STORAGE_ACCOUNT_KEY ` , or the client Iceberg will use the above static secret keys. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Improvement] When a malformed name is passed to the CLI command to list columns in a table an exception occurs [gravitino]
Abyss-lord commented on issue #5808: URL: https://github.com/apache/gravitino/issues/5808#issuecomment-2533559910 Hi Justin, Could you please assign this issue to me? I would like to work on it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5624] feat(bundles): support ADLS credential provider [gravitino]
FANNG1 commented on PR #5737: URL: https://github.com/apache/gravitino/pull/5737#issuecomment-2533569286 The main problem of the current implementation is passing Azure secret key not the token(token name is not correct) to the client side, The `StorageSharedKeyCredential` key problems exists in the client side not the Iceberg REST server side, which means we could update the Iceberg version when testing. or update the Iceberg version in the server side is also ok but I prefer to do it in another PR -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5778] feat(aliyun-bundles)support OSS secret key credential [gravitino]
sunxiaojian commented on code in PR #5814: URL: https://github.com/apache/gravitino/pull/5814#discussion_r1879293167 ## api/src/main/java/org/apache/gravitino/credential/OSSSecretKeyCredential.java: ## @@ -0,0 +1,116 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.credential; + +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableMap; +import java.util.Map; +import org.apache.commons.lang3.StringUtils; + +/** OSS secret key credential. */ +public class OSSSecretKeyCredential implements Credential { + + /** OSS secret key credential type. */ + public static final String OSS_SECRET_KEY_CREDENTIAL_TYPE = "oss-secret-key"; + /** The static access key ID used to access OSS data. */ + public static final String GRAVITINO_OSS_STATIC_ACCESS_KEY_ID = "oss-access-key-id"; + /** The static secret access key used to access OSS data. */ + public static final String GRAVITINO_OSS_STATIC_SECRET_ACCESS_KEY = "oss-secret-access-key"; + + private String accessKeyId; + private String secretAccessKey; + + /** + * Constructs an instance of {@link OSSSecretKeyCredential} with the static OSS access key ID and + * secret access key. + * + * @param accessKeyId The OSS static access key ID. + * @param secretAccessKey The OSS static secret access key. + */ + public OSSSecretKeyCredential(String accessKeyId, String secretAccessKey) { +Preconditions.checkNotNull(accessKeyId, "OSS access key Id should not null"); Review Comment: fixed -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] [Minor] Improve misisng name error message [gravitino]
justinmclean opened a new pull request, #5823: URL: https://github.com/apache/gravitino/pull/5823 ### What changes were proposed in this pull request? Explain to the user what is missing and how to fix it. ### Why are the changes needed? See above. Fix: # N/A ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Locally. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[I] [Improvement] Dropping a metalake via the Gravitino CLI gives an "in use" exception. [gravitino]
justinmclean opened a new issue, #5826: URL: https://github.com/apache/gravitino/issues/5826 ### What would you like to be improved? Looks like there has been some code changes around metalake being in use that make it difficult to drop a metalake. If you create a metalake called `my_metalake` and run this command `gcli metalake delete --metalake my_metalake` you get an exception. The exection info is misleading `Failed to operate metalake(s) [my_metalake] operation [DROP], reason [Metalake my_metalake is in use, please disable it first or use force option]` `gcli metalake delete --metalake my_metalake --force` gives the same error. ### How should we improve? Look into code changes and come up with a way to fix this, and also possibly fix the misleading exception text. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
(gravitino) branch main updated (692dfb409 -> 31d3b4a6c)
This is an automated email from the ASF dual-hosted git repository. jmclean pushed a change to branch main in repository https://gitbox.apache.org/repos/asf/gravitino.git from 692dfb409 [#5796][#5798] Add e2e test case for creating schema/fileset/topic/table (#5805) add 31d3b4a6c [#5807] improvement(CLI): Add functionality to display help information when no args are passed (#5821) No new revisions were added by this update. Summary of changes: clients/cli/src/main/java/org/apache/gravitino/cli/Main.java | 4 1 file changed, 4 insertions(+)
Re: [I] [Improvement] The Gravitino CLI shoudl show help if no arguments are passed [gravitino]
justinmclean closed issue #5807: [Improvement] The Gravitino CLI shoudl show help if no arguments are passed URL: https://github.com/apache/gravitino/issues/5807 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5807] improvement(CLI): Add functionality to display help information when no args are passed [gravitino]
justinmclean merged PR #5821: URL: https://github.com/apache/gravitino/pull/5821 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Improvement] The Gravitino CLI shoudl show help if no arguments are passed [gravitino]
justinmclean closed issue #5807: [Improvement] The Gravitino CLI shoudl show help if no arguments are passed URL: https://github.com/apache/gravitino/issues/5807 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Improvement] Dropping a metalake via the Gravitino CLI gives an "in use" exception. [gravitino]
justinmclean commented on issue #5826: URL: https://github.com/apache/gravitino/issues/5826#issuecomment-2533608170 A similar error occurs when deleting catalogs -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Subtask] Support distributions expression [gravitino]
SophieTech88 commented on issue #5729: URL: https://github.com/apache/gravitino/issues/5729#issuecomment-2533622965 Can I work on this issue? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Subtask] Support sorts expression [gravitino]
SophieTech88 commented on issue #5730: URL: https://github.com/apache/gravitino/issues/5730#issuecomment-2533623035 Can I work on this issue? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Subtask] Support transforms expression [gravitino]
SophieTech88 commented on issue #5732: URL: https://github.com/apache/gravitino/issues/5732#issuecomment-2533623113 Can I work on this issue? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5745] feat(CLI): Table format output for ListCatalogs command [gravitino]
waukin commented on code in PR #5759: URL: https://github.com/apache/gravitino/pull/5759#discussion_r1879329596 ## clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java: ## @@ -159,17 +159,18 @@ private void handleMetalakeCommand() { FullName name = new FullName(line); String metalake = name.getMetalakeName(); String outputFormat = line.getOptionValue(GravitinoOptions.OUTPUT); +ListOptions listOptions = new ListOptions(outputFormat); Review Comment: So you mean we don't need a `ListOptions` wrapper at this stage because we currently have only one display option, namely outputFormat? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[I] [Improvement] Missing tag name in creating Gravitino CLI command gives an excpetion. [gravitino]
justinmclean opened a new issue, #5830: URL: https://github.com/apache/gravitino/issues/5830 ### What would you like to be improved? Running the command `gcli tag create --metalake metalake_demo` gives an exception. The same happens for `gcli tag details --metalake metalake_demo` and `gcli tag delete --metalake metalake_demo` ### How should we improve? A friendly error message should be displayed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Improvement] Creating a role in the Gravitino CLi with a missing role name give an unexpected error [gravitino]
justinmclean commented on issue #5832: URL: https://github.com/apache/gravitino/issues/5832#issuecomment-2533649447 Note the error is saying that the name of the of the role is empty so it not incorrect it's just not user-friendly. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Improvement] Slightly missing leading error message when user is not specified in Gravitino CLI [gravitino]
justinmclean commented on issue #5828: URL: https://github.com/apache/gravitino/issues/5828#issuecomment-2533652492 Note the error is saying that the name of the of the user is empty so it not incorrect it's just not user-friendly. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5745] feat(CLI): Table format output for ListCatalogs command [gravitino]
justinmclean commented on code in PR #5759: URL: https://github.com/apache/gravitino/pull/5759#discussion_r1879358547 ## clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java: ## @@ -159,17 +159,18 @@ private void handleMetalakeCommand() { FullName name = new FullName(line); String metalake = name.getMetalakeName(); String outputFormat = line.getOptionValue(GravitinoOptions.OUTPUT); +ListOptions listOptions = new ListOptions(outputFormat); Review Comment: I think so, it adds extra complexity and decreases code readability for little or no benefit. I think it would be useful if we had 2 or 3 options. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5602] feat(core): Add model storage schema layout (part-2) [gravitino]
yuqi1129 commented on code in PR #5728: URL: https://github.com/apache/gravitino/pull/5728#discussion_r1878121178 ## core/src/main/java/org/apache/gravitino/storage/relational/service/ModelVersionMetaService.java: ## @@ -0,0 +1,288 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.storage.relational.service; + +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.Multimap; +import java.io.IOException; +import java.util.Collections; +import java.util.List; +import java.util.Locale; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; +import org.apache.gravitino.Entity; +import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.Namespace; +import org.apache.gravitino.exceptions.NoSuchEntityException; +import org.apache.gravitino.meta.ModelEntity; +import org.apache.gravitino.meta.ModelVersionEntity; +import org.apache.gravitino.storage.relational.mapper.ModelMetaMapper; +import org.apache.gravitino.storage.relational.mapper.ModelVersionAliasRelMapper; +import org.apache.gravitino.storage.relational.mapper.ModelVersionMetaMapper; +import org.apache.gravitino.storage.relational.po.ModelPO; +import org.apache.gravitino.storage.relational.po.ModelVersionAliasRelPO; +import org.apache.gravitino.storage.relational.po.ModelVersionPO; +import org.apache.gravitino.storage.relational.utils.ExceptionUtils; +import org.apache.gravitino.storage.relational.utils.POConverters; +import org.apache.gravitino.storage.relational.utils.SessionUtils; +import org.apache.gravitino.utils.NameIdentifierUtil; +import org.apache.gravitino.utils.NamespaceUtil; +import org.glassfish.jersey.internal.guava.Lists; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class ModelVersionMetaService { + + private static final Logger LOG = LoggerFactory.getLogger(ModelVersionMetaService.class); + + private static final ModelVersionMetaService INSTANCE = new ModelVersionMetaService(); + + public static ModelVersionMetaService getInstance() { +return INSTANCE; + } + + private ModelVersionMetaService() {} + + public List listModelVersionsByNamespace(Namespace ns) { +NamespaceUtil.checkModelVersion(ns); + +NameIdentifier modelIdent = NameIdentifier.of(ns.levels()); +// Will throw a NoSuchEntityException if the model does not exist. +ModelEntity modelEntity = ModelMetaService.getInstance().getModelByIdentifier(modelIdent); + +List modelVersionPOs = +SessionUtils.getWithoutCommit( +ModelVersionMetaMapper.class, +mapper -> mapper.listModelVersionMetasByModelId(modelEntity.id())); + +if (modelVersionPOs.isEmpty()) { + return Collections.emptyList(); +} + +// Get the aliases for all the model versions. +List aliasRelPOs = +SessionUtils.getWithoutCommit( +ModelVersionAliasRelMapper.class, +mapper -> mapper.selectModelVersionAliasRelByModelId(modelEntity.id())); +Multimap aliasRelPOsByModelVersion = +ArrayListMultimap.create(); +aliasRelPOs.forEach(r -> aliasRelPOsByModelVersion.put(r.getModelVersion(), r)); + +return modelVersionPOs.stream() +.map( +m -> { + List versionAliasRelPOs = + Lists.newArrayList(aliasRelPOsByModelVersion.get(m.getModelVersion())); + return POConverters.fromModelVersionPO(modelIdent, m, versionAliasRelPOs); +}) +.collect(Collectors.toList()); + } + + public ModelVersionEntity getModelVersionByIdentifier(NameIdentifier ident) { +NameIdentifierUtil.checkModelVersion(ident); + +NameIdentifier modelIdent = NameIdentifier.of(ident.namespace().levels()); +// Will throw a NoSuchEntityException if the model does not exist. +ModelEntity modelEntity = ModelMetaService.getInstance().getModelByIdentifier(modelIdent); + +Integer modelVersion; +try { + modelVersion = Integer.valueOf(ident.name()); +} catch (NumberFormatException e) { + modelVersion = null; +} + +final Integer copiedModelVersion = modelVersion; +ModelVersionPO modelVersionPO = +SessionUt
Re: [PR] [#5745] feat(CLI): Table format output for ListCatalogs command [gravitino]
justinmclean commented on code in PR #5759: URL: https://github.com/apache/gravitino/pull/5759#discussion_r1878824511 ## clients/cli/src/main/java/org/apache/gravitino/cli/commands/CatalogDetails.java: ## @@ -35,14 +36,17 @@ public class CatalogDetails extends Command { * * @param url The URL of the Gravitino server. * @param ignoreVersions If true don't check the client/server versions match. - * @param outputFormat The output format. + * @param listOptions The list options. Review Comment: The name of this variable is confusing. You have also removed useful information i.e. that it controls teh output format. ## clients/cli/src/main/java/org/apache/gravitino/cli/commands/CatalogDetails.java: ## @@ -35,14 +36,17 @@ public class CatalogDetails extends Command { * * @param url The URL of the Gravitino server. * @param ignoreVersions If true don't check the client/server versions match. - * @param outputFormat The output format. + * @param listOptions The list options. Review Comment: The name of this variable is confusing. You have also removed useful information i.e. that it controls the output format. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5745] feat(CLI): Table format output for ListCatalogs command [gravitino]
justinmclean commented on code in PR #5759: URL: https://github.com/apache/gravitino/pull/5759#discussion_r1878833785 ## clients/cli/src/main/java/org/apache/gravitino/cli/commands/ListCatalogs.java: ## @@ -35,29 +36,26 @@ public class ListCatalogs extends Command { * @param url The URL of the Gravitino server. * @param ignoreVersions If true don't check the client/server versions match. * @param metalake The name of the metalake. + * @param listOptions The list options. */ - public ListCatalogs(String url, boolean ignoreVersions, String metalake) { -super(url, ignoreVersions); + public ListCatalogs( + String url, boolean ignoreVersions, String metalake, ListOptions listOptions) { +super(url, ignoreVersions, listOptions); this.metalake = metalake; } /** Lists all catalogs in a metalake. */ @Override public void handle() { -String[] catalogs = new String[0]; +Catalog[] catalogs; try { GravitinoClient client = buildClient(metalake); - catalogs = client.listCatalogs(); + catalogs = client.listCatalogsInfo(); + output(catalogs); Review Comment: This command is a list command that only lists the catalog names. The details command lists more information. This is now inconsistent with other commands. Why was this changed? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5569] improvement(CLI): Add ability to grant or revoke multiple roles or groups at once in the Gravitino CLI [gravitino]
justinmclean merged PR #5811: URL: https://github.com/apache/gravitino/pull/5811 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
(gravitino) branch main updated: [#5569] improvement(CLI): Add ability to grant or revoke multiple roles or groups at once in the Gravitino CLI (#5811)
This is an automated email from the ASF dual-hosted git repository. jmclean pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/gravitino.git The following commit(s) were added to refs/heads/main by this push: new 5bc8fefe3 [#5569] improvement(CLI): Add ability to grant or revoke multiple roles or groups at once in the Gravitino CLI (#5811) 5bc8fefe3 is described below commit 5bc8fefe381934aba25fb2a7f27370bbd747e516 Author: Lord of Abyss <103809695+abyss-l...@users.noreply.github.com> AuthorDate: Wed Dec 11 05:33:33 2024 +0800 [#5569] improvement(CLI): Add ability to grant or revoke multiple roles or groups at once in the Gravitino CLI (#5811) ### What changes were proposed in this pull request? Add ability to grant or revoke multiple roles or groups at once in the Gravitino CLI, ### Why are the changes needed? Fix:[#5569](https://github.com/apache/gravitino/issues/5569) ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? I. bash test **Grant\revoke role to user** 1. Grant single role to a user  2. Grant multiple role to a user  3. Revoke multiple role to from user  **Grant\revoke role to group** 1. Grant single role to a group  2. Grant multiple role to a group  3. Revoke multiple role to from group  II. unit test Please check the [commit](https://github.com/apache/gravitino/commit/788396aab379638707ca5733e89069fab0e0002f) --- .../apache/gravitino/cli/GravitinoCommandLine.java | 23 +--- .../org/apache/gravitino/cli/GravitinoOptions.java | 4 +- .../apache/gravitino/cli/TestGroupCommands.java| 66 - .../org/apache/gravitino/cli/TestUserCommands.java | 67 ++ 4 files changed, 148 insertions(+), 12 deletions(-) diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java b/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java index 58255849a..843593487 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java @@ -19,6 +19,7 @@ package org.apache.gravitino.cli; +import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import java.io.BufferedReader; import java.io.IOException; @@ -49,6 +50,8 @@ public class GravitinoCommandLine extends TestableCommandLine { public static final String CMD = "gcli"; // recommended name public static final String DEFAULT_URL = "http://localhost:8090";; + // This joiner is used to join multiple outputs to be displayed, e.g. roles or groups + private static final Joiner COMMA_JOINER = Joiner.on(", ").skipNulls(); /** * Gravitino Command line. @@ -382,15 +385,17 @@ public class GravitinoCommandLine extends TestableCommandLine { boolean force = line.hasOption(GravitinoOptions.FORCE); newDeleteUser(url, ignore, force, metalake, user).handle(); } else if (CommandActions.REVOKE.equals(command)) { - String role = line.getOptionValue(GravitinoOptions.ROLE); - if (role != null) { + String[] roles = line.getOptionValues(GravitinoOptions.ROLE); + for (String role : roles) { newRemoveRoleFromUser(url, ignore, metalake, user, role).handle(); } + System.out.printf("Remove roles %s from user %s%n", COMMA_JOINER.join(roles), user); } else if (CommandActions.GRANT.equals(command)) { - String role = line.getOptionValue(GravitinoOptions.ROLE); - if (role != null) { + String[] roles = line.getOptionValues(GravitinoOptions.ROLE); + for (String role : roles) { newAddRoleToUser(url, ignore, metalake, user, role).handle(); } + System.out.printf("Grant roles %s to user %s%n", String.join(", ", roles), user); } else { System.err.println(ErrorMessages.UNSUPPORTED_ACTION); } @@ -417,15 +422,17 @@ public class GravitinoCommandLine extends TestableCommandLine { boolean force = line.hasOption(GravitinoOptions.FORCE); newDeleteGroup(url, ignore, force, metalake, group).handle(); } else if (CommandActions.REVOKE.equals(command)) { - String role = line.getOptionValue(GravitinoOption
Re: [PR] feat(helm-chart): remove helm-chart support [gravitino-playground]
unknowntpo commented on code in PR #110: URL: https://github.com/apache/gravitino-playground/pull/110#discussion_r1871447104 ## healthcheck/gravitino-healthcheck.sh: ## @@ -23,10 +23,8 @@ max_attempts=3 attempt=0 success=false -HOST_IP=${GRAVITINO_HOST_IP:-localhost} - while [ $attempt -lt $max_attempts ]; do - response=$(curl -X GET -H "Content-Type: application/json" http://${HOST_IP}:8090/api/version) + response=$(curl -X GET -H "Content-Type: application/json" http://gravitino:8090/api/version) Review Comment: yeah, reverted to `127.0.0.1` at f4404e6ec2d83b07d04f5e04ce38f3634733c2d0. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat(helm-chart): remove helm-chart support [gravitino-playground]
unknowntpo commented on code in PR #110: URL: https://github.com/apache/gravitino-playground/pull/110#discussion_r1879147916 ## init/hive/init.sh: ## @@ -21,7 +21,6 @@ sed -i '$d' /usr/local/sbin/start.sh sed -i '$d' /usr/local/sbin/start.sh cp /tmp/hive/core-site.xml /tmp/hadoop-conf -sed -i "s|hdfs://localhost:9000|hdfs://${HIVE_HOST_IP}:9000|g" /usr/local/hive/conf/hive-site.xml /bin/bash /usr/local/sbin/start.sh Review Comment: I changed it to ``` sed -i -E 's/tail -f \/dev\/null/\s/g' /usr/local/sbin/start.sh ``` , since last line might be a blank string. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat(helm-chart): remove helm-chart support [gravitino-playground]
unknowntpo commented on code in PR #110: URL: https://github.com/apache/gravitino-playground/pull/110#discussion_r1879147916 ## init/hive/init.sh: ## @@ -21,7 +21,6 @@ sed -i '$d' /usr/local/sbin/start.sh sed -i '$d' /usr/local/sbin/start.sh cp /tmp/hive/core-site.xml /tmp/hadoop-conf -sed -i "s|hdfs://localhost:9000|hdfs://${HIVE_HOST_IP}:9000|g" /usr/local/hive/conf/hive-site.xml /bin/bash /usr/local/sbin/start.sh Review Comment: I changed it to ``` sed -i -E 's/tail -f \/dev\/null/\s/g' /usr/local/sbin/start.sh ``` , since last line might be a blank string. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat(helm-chart): remove helm-chart support [gravitino-playground]
unknowntpo commented on code in PR #110: URL: https://github.com/apache/gravitino-playground/pull/110#discussion_r1879148347 ## init/hive/init.sh: ## @@ -21,7 +21,6 @@ sed -i '$d' /usr/local/sbin/start.sh sed -i '$d' /usr/local/sbin/start.sh Review Comment: I changed it to ``` sed -i -E 's/tail -f \/dev\/null/\s/g' /usr/local/sbin/start.sh ``` , since last line might be a blank string. fixed at 7ed9dae9db5836fd37272d3c680777972c5051b2 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat(helm-chart): remove helm-chart support [gravitino-playground]
unknowntpo commented on code in PR #110: URL: https://github.com/apache/gravitino-playground/pull/110#discussion_r1871447898 ## healthcheck/trino-healthcheck.sh: ## @@ -20,7 +20,7 @@ set -ex # Because trino-connector must first synchronize a default metalake from the Gravitino server -response=$(trino --server ${TRINO_HOST_IP}:8080 --execute "SHOW CATALOGS LIKE 'catalog_hive'") +response=$(trino --server trino:8080 --execute "SHOW CATALOGS LIKE 'catalog_hive'") Review Comment: sure, I changed this to `localhost`. 1640e8f2111a162f310da6abe8442a43bcae9615 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#3888] Metadata Support ClickHouse [gravitino]
flaming-archer commented on code in PR #5819: URL: https://github.com/apache/gravitino/pull/5819#discussion_r1879258818 ## catalogs/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operation/ClickHouseDatabaseOperations.java: ## @@ -0,0 +1,95 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.catalog.clickhouse.operation; + +import com.google.common.collect.ImmutableSet; +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import org.apache.gravitino.catalog.jdbc.operation.JdbcDatabaseOperations; +import org.apache.gravitino.exceptions.NoSuchSchemaException; + +/** Database operations for ClickHouse. */ +public class ClickHouseDatabaseOperations extends JdbcDatabaseOperations { + + @Override + protected boolean supportSchemaComment() { +return false; + } + + @Override + protected Set createSysDatabaseNameSet() { +return ImmutableSet.of("information_schema", "INFORMATION_SCHEMA", "default", "system"); + } + + @Override + public boolean delete(String databaseName, boolean cascade) { +LOG.info("Beginning to drop database {}", databaseName); +try { + dropDatabase(databaseName, cascade); + LOG.info("Finished dropping database {}", databaseName); +} catch (NoSuchSchemaException e) { + return false; +} catch (Exception e) { + if (e.getMessage() != null + && (e.getMessage().contains("Database " + databaseName + " does not exist.") + || e.getMessage().contains("Database `" + databaseName + "` does not exist."))) { +return false; + } + + if (e.getMessage() != null + && e.getMessage() + .contains( + "Database " + + databaseName + + " is not empty, the value of cascade should be true.")) { +throw e; + } +} +return true; Review Comment: > For methods like this, we may want to think twice about the exit condition. There are three possible outcomes: > > * `true`: everything went smooth, great; > * `false`: something is bad, operation failed; > * Exception: this is the gotcha for the caller. This means ... well, something bad > happened, the operation failed. > > A caller has to handle two potential results, and the caller has to be aware of the exception. This logic is not maintainable. Maybe there are two alternatives here: > > * Remove the Exception, let the function return ether `true` or `false`. > That means we don't throw exceptions. > * Remove the return value, the function either succeed, or it throws an exception. > The caller is supposed to do a `try ... catch`. > > I'm leaning toward the first one, but it is up to your decision. I also lean towards the first option. The behavior of the test cases I tested at that time was very strange, so I will try to change it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5685] Change main command section of Gravitino CLI to use switch statements [gravitino]
tengqm commented on code in PR #5793: URL: https://github.com/apache/gravitino/pull/5793#discussion_r1879129401 ## api/src/main/java/org/apache/gravitino/SupportsCatalogs.java: ## @@ -98,6 +101,23 @@ Catalog createCatalog( Map properties) throws NoSuchMetalakeException, CatalogAlreadyExistsException; + /** + * Create a managed catalog with specified catalog name, type, comment, and properties. + * + * @param catalogName the name of the catalog. + * @param type the type of the catalog. + * @param comment the comment of the catalog. + * @param properties the properties of the catalog. + * @return The created catalog. + * @throws NoSuchMetalakeException If the metalake does not exist. + * @throws CatalogAlreadyExistsException If the catalog already exists. + */ + default Catalog createCatalog( + String catalogName, Catalog.Type type, String comment, Map properties) + throws NoSuchMetalakeException, CatalogAlreadyExistsException { +return createCatalog(catalogName, type, null, comment, properties); Review Comment: ```suggestion String name, Catalog.Type type, String comment, Map properties) throws NoSuchMetalakeException, CatalogAlreadyExistsException { return createCatalog(name, type, null, comment, properties); ``` ## api/src/main/java/org/apache/gravitino/credential/GCSTokenCredential.java: ## @@ -70,4 +83,11 @@ public Map credentialInfo() { public String token() { return token; } + + private void validate(String token, long expireTimeInMs) { Review Comment: How about embed this method into `initialize`? The GCSTokenCredential is immutable IIUC, so I suppose that this validation would be invoked only from `initialize`. We can move this out when this assumption doesn't hold in the future. ## clients/client-java/src/main/java/org/apache/gravitino/client/ErrorHandlers.java: ## @@ -858,6 +868,43 @@ public void accept(ErrorResponse errorResponse) { } } + /** Error handler specific to Credential operations. */ + @SuppressWarnings("FormatStringAnnotation") + private static class CredentialErrorHandler extends RestErrorHandler { + +private static final CredentialErrorHandler INSTANCE = new CredentialErrorHandler(); + +@Override +public void accept(ErrorResponse errorResponse) { + String errorMessage = formatErrorMessage(errorResponse); + + switch (errorResponse.getCode()) { +case ErrorConstants.ILLEGAL_ARGUMENTS_CODE: + throw new IllegalArgumentException(errorMessage); + +case ErrorConstants.NOT_FOUND_CODE: + if (errorResponse.getType().equals(NoSuchMetalakeException.class.getSimpleName())) { +throw new NoSuchMetalakeException(errorMessage); + } else if (errorResponse Review Comment: Em ... I don't think we want to make this exception an explicit one. The reason is that we may intentionally make this less clear, for security's sake. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5745] feat(CLI): Table format output for ListCatalogs command [gravitino]
justinmclean commented on code in PR #5759: URL: https://github.com/apache/gravitino/pull/5759#discussion_r1878816436 ## clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java: ## @@ -208,11 +209,12 @@ private void handleCatalogCommand() { FullName name = new FullName(line); String metalake = name.getMetalakeName(); String outputFormat = line.getOptionValue(GravitinoOptions.OUTPUT); +ListOptions listOptions = new ListOptions(outputFormat); Command.setAuthenticationMode(auth, userName); if (CommandActions.LIST.equals(command)) { - newListCatalogs(url, ignore, metalake).handle(); + newListCatalogs(url, ignore, metalake, listOptions).handle(); Review Comment: The order of the parameters should be the same, other calls have listOptions before metalake. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5602] feat(core): Add model storage schema layout (part-2) [gravitino]
jerryshao commented on code in PR #5728: URL: https://github.com/apache/gravitino/pull/5728#discussion_r1879217367 ## core/src/main/java/org/apache/gravitino/storage/relational/service/ModelVersionMetaService.java: ## @@ -0,0 +1,288 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.storage.relational.service; + +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.Multimap; +import java.io.IOException; +import java.util.Collections; +import java.util.List; +import java.util.Locale; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; +import org.apache.gravitino.Entity; +import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.Namespace; +import org.apache.gravitino.exceptions.NoSuchEntityException; +import org.apache.gravitino.meta.ModelEntity; +import org.apache.gravitino.meta.ModelVersionEntity; +import org.apache.gravitino.storage.relational.mapper.ModelMetaMapper; +import org.apache.gravitino.storage.relational.mapper.ModelVersionAliasRelMapper; +import org.apache.gravitino.storage.relational.mapper.ModelVersionMetaMapper; +import org.apache.gravitino.storage.relational.po.ModelPO; +import org.apache.gravitino.storage.relational.po.ModelVersionAliasRelPO; +import org.apache.gravitino.storage.relational.po.ModelVersionPO; +import org.apache.gravitino.storage.relational.utils.ExceptionUtils; +import org.apache.gravitino.storage.relational.utils.POConverters; +import org.apache.gravitino.storage.relational.utils.SessionUtils; +import org.apache.gravitino.utils.NameIdentifierUtil; +import org.apache.gravitino.utils.NamespaceUtil; +import org.glassfish.jersey.internal.guava.Lists; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class ModelVersionMetaService { + + private static final Logger LOG = LoggerFactory.getLogger(ModelVersionMetaService.class); + + private static final ModelVersionMetaService INSTANCE = new ModelVersionMetaService(); + + public static ModelVersionMetaService getInstance() { +return INSTANCE; + } + + private ModelVersionMetaService() {} + + public List listModelVersionsByNamespace(Namespace ns) { +NamespaceUtil.checkModelVersion(ns); + +NameIdentifier modelIdent = NameIdentifier.of(ns.levels()); +// Will throw a NoSuchEntityException if the model does not exist. +ModelEntity modelEntity = ModelMetaService.getInstance().getModelByIdentifier(modelIdent); + +List modelVersionPOs = +SessionUtils.getWithoutCommit( +ModelVersionMetaMapper.class, +mapper -> mapper.listModelVersionMetasByModelId(modelEntity.id())); + +if (modelVersionPOs.isEmpty()) { + return Collections.emptyList(); +} + +// Get the aliases for all the model versions. +List aliasRelPOs = +SessionUtils.getWithoutCommit( +ModelVersionAliasRelMapper.class, +mapper -> mapper.selectModelVersionAliasRelByModelId(modelEntity.id())); +Multimap aliasRelPOsByModelVersion = +ArrayListMultimap.create(); +aliasRelPOs.forEach(r -> aliasRelPOsByModelVersion.put(r.getModelVersion(), r)); + +return modelVersionPOs.stream() +.map( +m -> { + List versionAliasRelPOs = + Lists.newArrayList(aliasRelPOsByModelVersion.get(m.getModelVersion())); + return POConverters.fromModelVersionPO(modelIdent, m, versionAliasRelPOs); +}) +.collect(Collectors.toList()); + } + + public ModelVersionEntity getModelVersionByIdentifier(NameIdentifier ident) { +NameIdentifierUtil.checkModelVersion(ident); + +NameIdentifier modelIdent = NameIdentifier.of(ident.namespace().levels()); +// Will throw a NoSuchEntityException if the model does not exist. +ModelEntity modelEntity = ModelMetaService.getInstance().getModelByIdentifier(modelIdent); + +Integer modelVersion; +try { + modelVersion = Integer.valueOf(ident.name()); +} catch (NumberFormatException e) { + modelVersion = null; +} + +final Integer copiedModelVersion = modelVersion; +ModelVersionPO modelVersionPO = +SessionU
Re: [PR] [#5734] feat (gvfs-fuse): Gvfs-fuse basic FUSE-level implementation and code structure layout [gravitino]
mchades commented on code in PR #5738: URL: https://github.com/apache/gravitino/pull/5738#discussion_r1879218669 ## .github/workflows/gvfs-fuse-build-test.yml: ## @@ -0,0 +1,93 @@ +name: Build gvfs-fuse and testing + +# Controls when the workflow will run +on: + push: +branches: [ "main", "branch-*" ] + pull_request: +branches: [ "main", "branch-*" ] + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + changes: +runs-on: ubuntu-latest +steps: + - uses: actions/checkout@v3 + - uses: dorny/paths-filter@v2 +id: filter +with: + filters: | +source_changes: + - .github/** + - api/** + - bin/** + - catalogs/** + - clients/filesystem-fuse/** + - common/** + - conf/** + - core/** + - dev/** + - gradle/** + - meta/** + - scripts/** + - server/** + - server-common/** + - build.gradle.kts + - gradle.properties + - gradlew + - setting.gradle.kts +outputs: + source_changes: ${{ steps.filter.outputs.source_changes }} + + # Build for AMD64 architecture + Gvfs-Build: +needs: changes +if: needs.changes.outputs.source_changes == 'true' +runs-on: ubuntu-latest +timeout-minutes: 60 +strategy: + matrix: +architecture: [linux/amd64] +java-version: [ 17 ] +env: + PLATFORM: ${{ matrix.architecture }} +steps: + - uses: actions/checkout@v3 + + - uses: actions/setup-java@v4 +with: + java-version: ${{ matrix.java-version }} + distribution: 'temurin' + cache: 'gradle' + + - name: Set up QEMU +uses: docker/setup-qemu-action@v2 + + - name: Check required command +run: | + dev/ci/check_commands.sh + + - name: Build and test Gravitino +run: | + ./gradlew :clients:filesystem-fuse:build -PenableFuse=true + + - name: Package Gravitino +run: | + ./gradlew compileDistribution -x test -PjdkVersion=${{ matrix.java-version }} -PenableFuse=true Review Comment: what's the function of `-PenableFuse=true` here -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5623] feat(python): supports credential API in python client [gravitino]
FANNG1 commented on code in PR #5777: URL: https://github.com/apache/gravitino/pull/5777#discussion_r1879219353 ## clients/client-python/gravitino/dto/responses/credential_response.py: ## @@ -0,0 +1,44 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from typing import List +from dataclasses import dataclass, field +from dataclasses_json import config + +from gravitino.dto.credential_dto import CredentialDTO +from gravitino.dto.responses.base_response import BaseResponse +from gravitino.exceptions.base import IllegalArgumentException + + +@dataclass +class CredentialResponse(BaseResponse): +"""Response for credential response.""" + +_credentials: List[CredentialDTO] = field(metadata=config(field_name="credentials")) + +def credentials(self) -> List[CredentialDTO]: +return self._credentials + +def validate(self): +"""Validates the response data. + +Raises: +IllegalArgumentException if credentials are None. +""" +if self._credentials is None: Review Comment: an empty `_credentials` is legal, please refer to the API of `SupportsCredentials` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5623] feat(python): supports credential API in python client [gravitino]
FANNG1 commented on code in PR #5777: URL: https://github.com/apache/gravitino/pull/5777#discussion_r1879218498 ## clients/client-python/gravitino/client/metadata_object_credential_operations.py: ## @@ -0,0 +1,67 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import logging +from typing import List +from gravitino.api.credential.supports_credentials import SupportsCredentials +from gravitino.api.credential.credential import Credential +from gravitino.api.metadata_object import MetadataObject +from gravitino.dto.credential_dto import CredentialDTO +from gravitino.dto.responses.credential_response import CredentialResponse +from gravitino.exceptions.handlers.credential_error_handler import ( +CREDENTIAL_ERROR_HANDLER, +) +from gravitino.utils import HTTPClient +from gravitino.utils.credential_utils import CredentialUtils + +logger = logging.getLogger(__name__) + + +class MetadataObjectCredentialOperations(SupportsCredentials): +def __init__( +self, +metalake_name: str, +metadata_object: MetadataObject, +rest_client: HTTPClient, +): +self._rest_client = rest_client +t = metadata_object.type().value Review Comment: updated -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[I] [Improvement] Improve error message for missing role name in the "gcli role create" command. [gravitino]
Gurleen12star opened a new issue, #5834: URL: https://github.com/apache/gravitino/issues/5834 ### What would you like to be improved? Currently, when the gcli role create command is run without specifying a role name, the error message shown is vague and does not directly indicate the missing field. This can cause confusion and difficulty in troubleshooting for users. ### How should we improve? The error message should be more descriptive and directly point out that the role name is missing. An example of the improved error message could be: Error: The role name is required and cannot be empty. Please use the '--role-name ' argument to specify the role name. This will help users easily identify what is missing and how to resolve the issue without needing to look up documentation or guess what went wrong. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5729]feat(client-python): Update the distribution expression in python [gravitino]
tengqm commented on code in PR #5833: URL: https://github.com/apache/gravitino/pull/5833#discussion_r1879474574 ## clients/client-python/gravitino/api/expressions/distributions/distribution.py: ## @@ -0,0 +1,58 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from gravitino.api.expressions.expression import Expression + + +class Distribution: Review Comment: If you are mimicking the Java implementation, you may want to make this an ABC class. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#3888] Metadata Support ClickHouse [gravitino]
tengqm commented on code in PR #5819: URL: https://github.com/apache/gravitino/pull/5819#discussion_r1879481898 ## catalogs/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/ClickHouseTypeConverter.java: ## @@ -0,0 +1,193 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.catalog.clickhouse.converter; + +import org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter; +import org.apache.gravitino.rel.types.Type; +import org.apache.gravitino.rel.types.Types; + +/** Type converter for ClickHouse. */ +public class ClickHouseTypeConverter extends JdbcTypeConverter { + + static final String INT8 = "Int8"; + static final String INT16 = "Int16"; + static final String INT32 = "Int32"; + static final String INT64 = "Int64"; + static final String INT128 = "Int128"; + static final String INT256 = "Int256"; + static final String UINT8 = "UInt8"; + static final String UINT16 = "UInt16"; + static final String UINT32 = "UInt32"; + static final String UINT64 = "UInt64"; + static final String UINT128 = "UInt128"; + static final String UINT256 = "UInt256"; + + static final String FLOAT32 = "Float32"; + static final String FLOAT64 = "Float64"; + static final String BFLOAT16 = "BFloat16"; + static final String DECIMAL = "Decimal"; + static final String STRING = "String"; + static final String FIXEDSTRING = "FixedString"; + static final String DATE = "Date"; + static final String DATE32 = "Date32"; + static final String DATETIME = "DateTime"; + static final String DATETIME64 = "DateTime64"; + static final String ENUM = "Enum"; + static final String BOOL = "Bool"; + static final String UUID = "UUID"; + + // bellow is Object Data Type + static final String IPV4 = "IPv4"; + static final String IPV6 = "IPv6"; + static final String ARRAY = "Array"; + static final String TUPLE = "Tuple"; + static final String MAP = "Map"; + static final String VARIANT = "Variant"; + static final String LOWCARDINALITY = "LowCardinality"; + static final String NULLABLE = "Nullable"; + static final String AGGREGATEFUNCTION = "AggregateFunction"; + static final String SIMPLEAGGREGATEFUNCTION = "SimpleAggregateFunction"; + static final String GEO = "Geo"; + + // bellow is Special Data Types + static final String Domains = "Domains"; + static final String Nested = "Nested"; + static final String Dynamic = "Dynamic"; + static final String JSON = "JSON"; + + @Override + public Type toGravitino(JdbcTypeBean typeBean) { +String typeName = typeBean.getTypeName(); +if (typeName.startsWith("Nullable(")) { + typeName = typeName.substring(9, typeName.length() - 1); +} + +if (typeName.startsWith("Decimal(")) { + typeName = "Decimal"; +} + +if (typeName.startsWith("FixedString(")) { + typeName = "FixedString"; +} + +switch (typeName) { + case INT8: +return Types.ByteType.get(); + case INT16: +return Types.ShortType.get(); + case INT32: +return Types.IntegerType.get(); + case INT64: +return Types.LongType.get(); + case UINT8: +return Types.ByteType.unsigned(); + case UINT16: +return Types.ShortType.unsigned(); + case UINT32: +return Types.IntegerType.unsigned(); + case UINT64: +return Types.LongType.unsigned(); + case FLOAT32: +return Types.FloatType.get(); + case FLOAT64: +return Types.DoubleType.get(); + case DECIMAL: +return Types.DecimalType.of(typeBean.getColumnSize(), typeBean.getScale()); + case STRING: +return Types.StringType.get(); + case FIXEDSTRING: +return Types.FixedCharType.of(typeBean.getColumnSize()); + case DATE: +return Types.DateType.get(); + case DATE32: +return Types.DateType.get(); + case DATETIME: +return Types.TimestampType.withoutTimeZone(); + case DATETIME64: +return Types.TimestampType.withoutTimeZone(); + case BOOL: +return Types.BooleanType.get(); + case UUID: +return Types.UUIDType.get(); + default: +return Types.ExternalType.of(typeBean.
Re: [PR] [#5602] feat(core): Add model storage schema layout (part-2) [gravitino]
jerryshao commented on code in PR #5728: URL: https://github.com/apache/gravitino/pull/5728#discussion_r1879486425 ## core/src/main/java/org/apache/gravitino/storage/relational/service/ModelVersionMetaService.java: ## @@ -0,0 +1,278 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.storage.relational.service; + +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.Multimap; +import java.io.IOException; +import java.util.Collections; +import java.util.List; +import java.util.Locale; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; +import org.apache.commons.lang3.math.NumberUtils; +import org.apache.gravitino.Entity; +import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.Namespace; +import org.apache.gravitino.exceptions.NoSuchEntityException; +import org.apache.gravitino.meta.ModelEntity; +import org.apache.gravitino.meta.ModelVersionEntity; +import org.apache.gravitino.storage.relational.mapper.ModelMetaMapper; +import org.apache.gravitino.storage.relational.mapper.ModelVersionAliasRelMapper; +import org.apache.gravitino.storage.relational.mapper.ModelVersionMetaMapper; +import org.apache.gravitino.storage.relational.po.ModelPO; +import org.apache.gravitino.storage.relational.po.ModelVersionAliasRelPO; +import org.apache.gravitino.storage.relational.po.ModelVersionPO; +import org.apache.gravitino.storage.relational.utils.ExceptionUtils; +import org.apache.gravitino.storage.relational.utils.POConverters; +import org.apache.gravitino.storage.relational.utils.SessionUtils; +import org.apache.gravitino.utils.NameIdentifierUtil; +import org.apache.gravitino.utils.NamespaceUtil; +import org.glassfish.jersey.internal.guava.Lists; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class ModelVersionMetaService { + + private static final Logger LOG = LoggerFactory.getLogger(ModelVersionMetaService.class); + + private static final ModelVersionMetaService INSTANCE = new ModelVersionMetaService(); + + public static ModelVersionMetaService getInstance() { +return INSTANCE; + } + + private ModelVersionMetaService() {} + + public List listModelVersionsByNamespace(Namespace ns) { +NamespaceUtil.checkModelVersion(ns); + +NameIdentifier modelIdent = NameIdentifier.of(ns.levels()); +// Will throw a NoSuchEntityException if the model does not exist. +ModelEntity modelEntity = ModelMetaService.getInstance().getModelByIdentifier(modelIdent); + +List modelVersionPOs = +SessionUtils.getWithoutCommit( +ModelVersionMetaMapper.class, +mapper -> mapper.listModelVersionMetasByModelId(modelEntity.id())); + +if (modelVersionPOs.isEmpty()) { + return Collections.emptyList(); +} + +// Get the aliases for all the model versions. +List aliasRelPOs = +SessionUtils.getWithoutCommit( +ModelVersionAliasRelMapper.class, +mapper -> mapper.selectModelVersionAliasRelsByModelId(modelEntity.id())); +Multimap aliasRelPOsByModelVersion = +ArrayListMultimap.create(); +aliasRelPOs.forEach(r -> aliasRelPOsByModelVersion.put(r.getModelVersion(), r)); + +return modelVersionPOs.stream() +.map( +m -> { + List versionAliasRelPOs = + Lists.newArrayList(aliasRelPOsByModelVersion.get(m.getModelVersion())); + return POConverters.fromModelVersionPO(modelIdent, m, versionAliasRelPOs); +}) +.collect(Collectors.toList()); + } + + public ModelVersionEntity getModelVersionByIdentifier(NameIdentifier ident) { +NameIdentifierUtil.checkModelVersion(ident); + +NameIdentifier modelIdent = NameIdentifier.of(ident.namespace().levels()); +// Will throw a NoSuchEntityException if the model does not exist. +ModelEntity modelEntity = ModelMetaService.getInstance().getModelByIdentifier(modelIdent); + +boolean isVersionNumber = NumberUtils.isCreatable(ident.name()); + +ModelVersionPO modelVersionPO = +SessionUtils.getWithoutCommit( +ModelVersionMetaMapper.class, +mapper -> { +
Re: [PR] [#5790] feat(core): Wildcard properties meta [gravitino]
mchades commented on code in PR #5791: URL: https://github.com/apache/gravitino/pull/5791#discussion_r1879492612 ## core/src/main/java/org/apache/gravitino/connector/WildcardPropertiesMetadata.java: ## @@ -0,0 +1,175 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.connector; + +import com.google.common.base.Preconditions; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Collectors; +import org.apache.gravitino.authorization.AuthorizationPropertiesMetadata; + +/** + * WildcardPropertiesMeta is a metadata interface for defining wildcard properties in the properties + * metadata. + * + * WildcardPropertiesMetadata interface defines: + * Prefix.Wildcard = "WildcardValue1,WildcardValue2" + * Prefix.*.properties-key1 = "" + * Prefix.*.properties-key2 = "" + * NOTE: Prefix name support multiple segment, separated by dot. for example: "a1.b1.c1" + * NOTE: Suffix properties-key{N} support multiple segment, separated by dot. for example: + * "x1.y1.z1" + * + * Use define a WildcardPropertiesMetadata object: + * a1.b1.c1.Wildcard = "WildcardValue1,WildcardValue2" + * a1.b1.c1.{WildcardValue1}.x1.y1.z1 = "WildcardValue1 property value" + * a1.b1.c1.{WildcardValue1}.x1.y2.z2 = "WildcardValue1 property value" + * a1.b1.c1.{WildcardValue1}.x1.y2.z3 = "WildcardValue1 property value" + * a1.b1.c1.{WildcardValue2}.x1.y1.z1 = "WildcardValue2 property value" + * a1.b1.c1.{WildcardValue2}.x1.y2.z2 = "WildcardValue2 property value" + * a1.b1.c1.{WildcardValue2}.x1.y2.z3 = "WildcardValue2 property value" + * + * Configuration Example: {@link AuthorizationPropertiesMetadata}, + * The Prefix is "authorization.chain", + * The Wildcard is "plugins" + * "authorization.chain.plugins" = "hive1,hdfs1" + * "authorization.chain.hive1.provider" = "ranger"; + * "authorization.chain.hive1.catalog-provider" = "hive"; + * "authorization.chain.hive1.ranger.auth.type" = "simple"; + * "authorization.chain.hive1.ranger.admin.url" = "http://localhost:6080";; + * "authorization.chain.hive1.ranger.username" = "admin"; + * "authorization.chain.hive1.ranger.password" = "admin"; + * "authorization.chain.hive1.ranger.service.name" = "hiveDev"; + * "authorization.chain.hdfs1.provider" = "ranger"; + * "authorization.chain.hdfs1.catalog-provider" = "hadoop"; + * "authorization.chain.hdfs1.ranger.auth.type" = "simple"; + * "authorization.chain.hdfs1.ranger.admin.url" = "http://localhost:6080";; + * "authorization.chain.hdfs1.ranger.username" = "admin"; + * "authorization.chain.hdfs1.ranger.password" = "admin"; + * "authorization.chain.hdfs1.ranger.service.name" = "hdfsDev"; + */ +public interface WildcardPropertiesMetadata { + String WILDCARD = "*"; + String WILDCARD_CONFIG_VALUES_SPLITTER = ","; + + /** The prefix name */ + String prefixName(); + + /** The WildcardNode define name */ + String wildcardName(); + + /** The `Prefix.Wildcard` properties key name */ + default String wildcardPropertyKey() { +return String.format("%s.%s", prefixName(), wildcardName()); + } + + /** Get the property value by wildcard value and property key */ + default String getPropertyValue(String wildcardValue, String propertyKey) { +return String.format("%s.%s.%s", prefixName(), wildcardValue, propertyKey); + } Review Comment: Are there more suitable names for these functions? It's confused now -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5790] feat(core): Wildcard properties meta [gravitino]
mchades commented on code in PR #5791: URL: https://github.com/apache/gravitino/pull/5791#discussion_r1879506799 ## core/src/main/java/org/apache/gravitino/connector/WildcardPropertiesMetadata.java: ## @@ -0,0 +1,175 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.connector; + +import com.google.common.base.Preconditions; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Collectors; +import org.apache.gravitino.authorization.AuthorizationPropertiesMetadata; + +/** + * WildcardPropertiesMeta is a metadata interface for defining wildcard properties in the properties + * metadata. + * + * WildcardPropertiesMetadata interface defines: + * Prefix.Wildcard = "WildcardValue1,WildcardValue2" + * Prefix.*.properties-key1 = "" + * Prefix.*.properties-key2 = "" + * NOTE: Prefix name support multiple segment, separated by dot. for example: "a1.b1.c1" + * NOTE: Suffix properties-key{N} support multiple segment, separated by dot. for example: + * "x1.y1.z1" + * + * Use define a WildcardPropertiesMetadata object: + * a1.b1.c1.Wildcard = "WildcardValue1,WildcardValue2" + * a1.b1.c1.{WildcardValue1}.x1.y1.z1 = "WildcardValue1 property value" + * a1.b1.c1.{WildcardValue1}.x1.y2.z2 = "WildcardValue1 property value" + * a1.b1.c1.{WildcardValue1}.x1.y2.z3 = "WildcardValue1 property value" + * a1.b1.c1.{WildcardValue2}.x1.y1.z1 = "WildcardValue2 property value" + * a1.b1.c1.{WildcardValue2}.x1.y2.z2 = "WildcardValue2 property value" + * a1.b1.c1.{WildcardValue2}.x1.y2.z3 = "WildcardValue2 property value" + * + * Configuration Example: {@link AuthorizationPropertiesMetadata}, + * The Prefix is "authorization.chain", + * The Wildcard is "plugins" + * "authorization.chain.plugins" = "hive1,hdfs1" + * "authorization.chain.hive1.provider" = "ranger"; + * "authorization.chain.hive1.catalog-provider" = "hive"; + * "authorization.chain.hive1.ranger.auth.type" = "simple"; + * "authorization.chain.hive1.ranger.admin.url" = "http://localhost:6080";; + * "authorization.chain.hive1.ranger.username" = "admin"; + * "authorization.chain.hive1.ranger.password" = "admin"; + * "authorization.chain.hive1.ranger.service.name" = "hiveDev"; + * "authorization.chain.hdfs1.provider" = "ranger"; + * "authorization.chain.hdfs1.catalog-provider" = "hadoop"; + * "authorization.chain.hdfs1.ranger.auth.type" = "simple"; + * "authorization.chain.hdfs1.ranger.admin.url" = "http://localhost:6080";; + * "authorization.chain.hdfs1.ranger.username" = "admin"; + * "authorization.chain.hdfs1.ranger.password" = "admin"; + * "authorization.chain.hdfs1.ranger.service.name" = "hdfsDev"; + */ +public interface WildcardPropertiesMetadata { + String WILDCARD = "*"; + String WILDCARD_CONFIG_VALUES_SPLITTER = ","; + + /** The prefix name */ + String prefixName(); + + /** The WildcardNode define name */ + String wildcardName(); + + /** The `Prefix.Wildcard` properties key name */ + default String wildcardPropertyKey() { +return String.format("%s.%s", prefixName(), wildcardName()); + } + + /** Get the property value by wildcard value and property key */ + default String getPropertyValue(String wildcardValue, String propertyKey) { +return String.format("%s.%s.%s", prefixName(), wildcardValue, propertyKey); + } + + /** + * Validate the wildcard properties in the properties metadata. + * + * @param propertiesMetadata the properties metadata + * @param properties the properties + * @throws IllegalArgumentException if the wildcard properties are not valid + */ + static void validate(PropertiesMetadata propertiesMetadata, Map properties) + throws IllegalArgumentException { +// Get all wildcard properties from PropertiesMetadata +List wildcardProperties = +propertiesMetadata.propertyEntries().keySet().stream() +.filter(propertiesMetadata::isWildcardProperty) +.collect(Collectors.toList()); +if (wildcardProperties.size() > 0) { Review Comment: suggest use: ```java if (wildcardProperties.isEmpty()) { return; } `
Re: [PR] [#5602] feat(core): Add model storage schema layout (part-2) [gravitino]
jerryshao commented on code in PR #5728: URL: https://github.com/apache/gravitino/pull/5728#discussion_r1879506711 ## core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/ModelMetaBaseSQLProvider.java: ## @@ -134,4 +134,11 @@ public String deleteModelMetasByLegacyTimeline( + ModelMetaMapper.TABLE_NAME + " WHERE deleted_at > 0 AND deleted_at < #{legacyTimeline} LIMIT #{limit}"; } + + public String updateModelLatestVersion(@Param("modelId") Long modelId) { +return "UPDATE " ++ ModelMetaMapper.TABLE_NAME ++ " SET model_latest_version = model_latest_version + 1" ++ " WHERE model_id = #{modelId} AND deleted_at = 0"; Review Comment: I change to use a transaction to handle this, can you please review again? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] Clickhouse 0.7 forpr [gravitino]
flaming-archer opened a new pull request, #5819: URL: https://github.com/apache/gravitino/pull/5819 ### Title 1. [#3888] feat(operator): support clickhouse" ### What changes were proposed in this pull request? Support clickhouse as a metadata. ### Why are the changes needed? No new api. Because ck is a new metadata, there has been a change in page classification. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#3888] Metadata Support ClickHouse [gravitino]
flaming-archer commented on PR #5819: URL: https://github.com/apache/gravitino/pull/5819#issuecomment-2532581924 @shaofengshi @xunliu Pls help to review this pr. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Bug report] The API for creating an Iceberg catalog has a deprecated parameter "warehouse" that is required in the HTTP payload [gravitino]
liuchunhao commented on issue #5756: URL: https://github.com/apache/gravitino/issues/5756#issuecomment-2531673638 Sure, I'd be happy to handle it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Improvement] Update Hadoop version from 3.1 to 3.3 to make fileset catalog up-to-date [gravitino]
yuqi1129 closed issue #5532: [Improvement] Update Hadoop version from 3.1 to 3.3 to make fileset catalog up-to-date URL: https://github.com/apache/gravitino/issues/5532 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Improvement] Update Hadoop version from 3.1 to 3.3 to make fileset catalog up-to-date [gravitino]
yuqi1129 commented on issue #5532: URL: https://github.com/apache/gravitino/issues/5532#issuecomment-2531698064 Close it as it has been fixed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5633] remove the key-value storage backend logic [gravitino]
yuqi1129 commented on PR #5645: URL: https://github.com/apache/gravitino/pull/5645#issuecomment-2531560854 Merge it into main, @pithecuse527 Thanks for your hard work. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5790] feat(core): Wildcard properties meta [gravitino]
xunliu commented on code in PR #5791: URL: https://github.com/apache/gravitino/pull/5791#discussion_r1877586827 ## core/src/main/java/org/apache/gravitino/authorization/AuthorizationPropertiesMetadata.java: ## @@ -0,0 +1,230 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.authorization; + +import com.google.common.collect.ImmutableMap; +import java.util.Map; +import org.apache.gravitino.connector.BasePropertiesMetadata; +import org.apache.gravitino.connector.PropertyEntry; +import org.apache.gravitino.connector.WildcardPropertiesMetadata; + +public class AuthorizationPropertiesMetadata extends BasePropertiesMetadata +implements WildcardPropertiesMetadata { + private static volatile AuthorizationPropertiesMetadata instance = null; + + public static synchronized AuthorizationPropertiesMetadata getInstance() { +if (instance == null) { + synchronized (AuthorizationPropertiesMetadata.class) { +if (instance == null) { + instance = new AuthorizationPropertiesMetadata(); +} + } +} +return instance; + } + + public static final String FIRST_SEGMENT_NAME = "authorization"; + public static final String SECOND_SEGMENT_NAME = "chain"; + + /** Ranger admin web URIs */ + private static final String RANGER_ADMIN_URL_KEY = "ranger.admin.url"; + + public static final String getRangerAdminUrlKey() { +return RANGER_ADMIN_URL_KEY; + } + + public static final String RANGER_ADMIN_URL = + String.format("%s.%s", FIRST_SEGMENT_NAME, RANGER_ADMIN_URL_KEY); + /** Ranger authentication type kerberos or simple */ + private static final String RANGER_AUTH_TYPE_KEY = "ranger.auth.type"; + + public static final String getRangerAuthTypeKey() { +return RANGER_AUTH_TYPE_KEY; + } + + public static final String RANGER_AUTH_TYPE = + String.format("%s.%s", FIRST_SEGMENT_NAME, RANGER_AUTH_TYPE_KEY); + /** + * Ranger admin web login username(auth_type=simple), or kerberos principal(auth_type=kerberos) + */ + private static final String RANGER_USERNAME_KEY = "ranger.username"; + + public static final String getRangerUsernameKey() { +return RANGER_USERNAME_KEY; + } + + public static final String RANGER_USERNAME = + String.format("%s.%s", FIRST_SEGMENT_NAME, RANGER_USERNAME_KEY); + /** + * Ranger admin web login user password(auth_type=simple), or path of the keytab + * file(auth_type=kerberos) + */ + private static final String RANGER_PASSWORD_KEY = "ranger.password"; + + public static final String getRangerPasswordKey() { +return RANGER_PASSWORD_KEY; + } + + public static final String RANGER_PASSWORD = + String.format("%s.%s", FIRST_SEGMENT_NAME, RANGER_PASSWORD_KEY); + + /** Ranger service name */ + private static final String RANGER_SERVICE_NAME_KEY = "ranger.service.name"; + + public static final String getRangerServiceNameKey() { +return RANGER_SERVICE_NAME_KEY; + } + + public static final String RANGER_SERVICE_NAME = + String.format("%s.%s", FIRST_SEGMENT_NAME, RANGER_SERVICE_NAME_KEY); + + /** Chain authorization plugin provider */ + private static final String CHAIN_CATALOG_PROVIDER_KEY = "catalog-provider"; + + public static final String getChainCatalogProviderKey() { +return CHAIN_CATALOG_PROVIDER_KEY; + } + + public static final String CHAIN_CATALOG_PROVIDER = + AuthorizationPropertiesMetadata.getInstance() + .getPropertyValue(Constants.WILDCARD, CHAIN_CATALOG_PROVIDER_KEY); + + /** Chain authorization plugin provider */ + private static final String CHAIN_PROVIDER_KEY = "provider"; + + public static final String getChainProviderKey() { +return CHAIN_PROVIDER_KEY; + } + + public static final String CHAIN_PROVIDER = + AuthorizationPropertiesMetadata.getInstance() + .getPropertyValue(Constants.WILDCARD, CHAIN_PROVIDER_KEY); + /** Chain authorization Ranger admin web URIs */ + public static final String CHAIN_RANGER_ADMIN_URL = + AuthorizationPropertiesMetadata.getInstance() + .getPropertyValue(Constants.WILDCARD, RANGER_ADMIN_URL_KEY); + /** Chain authorization Ranger authentication type kerberos or simple */ + public static fina
Re: [PR] [#5569] improvement(CLI): Add ability to grant or revoke multiple roles or groups at once in the Gravitino CLI [gravitino]
justinmclean commented on code in PR #5811: URL: https://github.com/apache/gravitino/pull/5811#discussion_r1877588930 ## clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java: ## @@ -100,12 +100,12 @@ public Options options() { options.addOption(createArgOption(AUTO, "column value auto-increments (true/false)")); options.addOption(createArgOption(DEFAULT, "default column value")); options.addOption(createSimpleOption("o", OWNER, "display entity owner")); -options.addOption(createArgOption("r", ROLE, "role name")); options.addOption(createArgOption(COLUMNFILE, "CSV file describing columns")); // Properties and tags can have multiple values Review Comment: This comment still needs to be changed -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5569] improvement(CLI): Add ability to grant or revoke multiple roles or groups at once in the Gravitino CLI [gravitino]
justinmclean commented on code in PR #5811: URL: https://github.com/apache/gravitino/pull/5811#discussion_r1877671578 ## clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java: ## @@ -100,12 +100,12 @@ public Options options() { options.addOption(createArgOption(AUTO, "column value auto-increments (true/false)")); options.addOption(createArgOption(DEFAULT, "default column value")); options.addOption(createSimpleOption("o", OWNER, "display entity owner")); -options.addOption(createArgOption("r", ROLE, "role name")); options.addOption(createArgOption(COLUMNFILE, "CSV file describing columns")); // Properties and tags can have multiple values Review Comment: As I said above, saying something like "Options that support multiple values" might be better. Once you have more than two things, making it more generic is better. If you were to change it your way, something like this would be better: "properties, tags and roles...". either would be fine. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5569] improvement(CLI): Add ability to grant or revoke multiple roles or groups at once in the Gravitino CLI [gravitino]
Abyss-lord commented on code in PR #5811: URL: https://github.com/apache/gravitino/pull/5811#discussion_r1877702909 ## clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java: ## @@ -100,12 +100,12 @@ public Options options() { options.addOption(createArgOption(AUTO, "column value auto-increments (true/false)")); options.addOption(createArgOption(DEFAULT, "default column value")); options.addOption(createSimpleOption("o", OWNER, "display entity owner")); -options.addOption(createArgOption("r", ROLE, "role name")); options.addOption(createArgOption(COLUMNFILE, "CSV file describing columns")); // Properties and tags can have multiple values Review Comment: Thank you Justin, I update the comment to `Options that support multiple values` and remove redundant comments > As I said above, saying something like "Options that support multiple values" might be better. Once you have more than two things, making it more generic is better. If you were to change it your way, something like this would be better: "properties, tags and roles...". either would be fine. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5734] feat (gvfs-fuse): Gvfs-fuse basic FUSE-level implementation and code structure layout [gravitino]
diqiu50 commented on code in PR #5738: URL: https://github.com/apache/gravitino/pull/5738#discussion_r1877717481 ## .github/workflows/gvfs-fuse-build-test.yml: ## @@ -0,0 +1,93 @@ +name: Build gvfs-fuse and testing + +# Controls when the workflow will run +on: + push: +branches: [ "main", "branch-*" ] + pull_request: +branches: [ "main", "branch-*" ] + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + changes: +runs-on: ubuntu-latest +steps: + - uses: actions/checkout@v3 + - uses: dorny/paths-filter@v2 +id: filter +with: + filters: | +source_changes: + - .github/** + - api/** + - bin/** + - catalogs/** + - clients/filesystem-fuse/** + - common/** + - conf/** + - core/** + - dev/** + - gradle/** + - meta/** + - scripts/** + - server/** + - server-common/** + - build.gradle.kts + - gradle.properties + - gradlew + - setting.gradle.kts +outputs: + source_changes: ${{ steps.filter.outputs.source_changes }} + + # Build for AMD64 architecture + Gvfs-Build: +needs: changes +if: needs.changes.outputs.source_changes == 'true' +runs-on: ubuntu-latest +timeout-minutes: 60 +strategy: + matrix: +architecture: [linux/amd64] +java-version: [ 17 ] +env: + PLATFORM: ${{ matrix.architecture }} +steps: + - uses: actions/checkout@v3 + + - uses: actions/setup-java@v4 +with: + java-version: ${{ matrix.java-version }} + distribution: 'temurin' + cache: 'gradle' + + - name: Set up QEMU +uses: docker/setup-qemu-action@v2 + + - name: Check required command +run: | + dev/ci/check_commands.sh + + - name: Build and test Gravitino +run: | + ./gradlew :clients:filesystem-fuse:build -PenableFuse=true + + - name: Package Gravitino +run: | + ./gradlew compileDistribution -x test -PjdkVersion=${{ matrix.java-version }} -PenableFuse=true Review Comment: No, fuseITs depend on the package of compileDistribution -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] [#5778] feat(aliyun-bundles)support OSS secret key credential [gravitino]
sunxiaojian opened a new pull request, #5814: URL: https://github.com/apache/gravitino/pull/5814 ### What changes were proposed in this pull request? Support OSS secret key credential ### Why are the changes needed? Fix: [# (5778)](https://github.com/apache/gravitino/issues/5778) ### How was this patch tested? IcebergRESTOSSSecretIT -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Improvement] OB-Catalog(WebUI): missing column data types [gravitino]
mchades commented on issue #5815: URL: https://github.com/apache/gravitino/issues/5815#issuecomment-2531243720 @LauraXia123 cc -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[I] [Improvement] OB-Catalog(WebUI): missing column data types [gravitino]
mchades opened a new issue, #5815: URL: https://github.com/apache/gravitino/issues/5815 ### What would you like to be improved? cannot create a ob table through web UI since: https://github.com/user-attachments/assets/e3d80946-7291-41ec-af78-7db50c221d08";> ### How should we improve? add data types -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5790] feat(core): Wildcard properties meta [gravitino]
xunliu commented on PR #5791: URL: https://github.com/apache/gravitino/pull/5791#issuecomment-2531121435 @mchades I fixed all the problems, please help me review it again. Thanks. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Bug report] http client out of time [gravitino]
an-shi-chi-fan commented on issue #5765: URL: https://github.com/apache/gravitino/issues/5765#issuecomment-2530876699 > may yes, it is. i found error log in hms -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] [#5815] fix(web): config oceanbase column types [gravitino]
LauraXia123 opened a new pull request, #5816: URL: https://github.com/apache/gravitino/pull/5816 ### What changes were proposed in this pull request? config oceanbase column types ### Why are the changes needed? N/A Fix: #5815 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? manually -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [MINOR] docs: fix typo in catalog.management exception message [gravitino]
diqiu50 merged PR #5812: URL: https://github.com/apache/gravitino/pull/5812 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
(gravitino) branch main updated: [MINOR] docs: fix typo in catalog.management exception message (#5812)
This is an automated email from the ASF dual-hosted git repository. diqiu50 pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/gravitino.git The following commit(s) were added to refs/heads/main by this push: new 53500a637 [MINOR] docs: fix typo in catalog.management exception message (#5812) 53500a637 is described below commit 53500a637dd500218ffdb9e08f361ca7de9a3cf6 Author: predator4ann AuthorDate: Tue Dec 10 19:17:13 2024 +0800 [MINOR] docs: fix typo in catalog.management exception message (#5812) ### What changes were proposed in this pull request? The exception message should be `catalog.management = dynamic` when the trino configuration is incorrect. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Not required Signed-off-by: zacsun --- .../main/java/org/apache/gravitino/trino/connector/GravitinoConfig.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConfig.java b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConfig.java index 36fecb11b..5f192fed9 100644 --- a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConfig.java +++ b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConfig.java @@ -237,7 +237,7 @@ public class GravitinoConfig { properties.getProperty(TRINO_CATALOG_MANAGEMENT))) { throw new TrinoException( GravitinoErrorCode.GRAVITINO_MISSING_CONFIG, - "Gravitino connector works only at catalog.management = static mode"); + "Gravitino connector works only at catalog.management = dynamic mode"); } } catch (IOException e) { throw new TrinoException(
Re: [PR] [#5569] improvement(CLI): Add ability to grant or revoke multiple roles or groups at once in the Gravitino CLI [gravitino]
Abyss-lord commented on code in PR #5811: URL: https://github.com/apache/gravitino/pull/5811#discussion_r1877626347 ## clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java: ## @@ -100,12 +100,12 @@ public Options options() { options.addOption(createArgOption(AUTO, "column value auto-increments (true/false)")); options.addOption(createArgOption(DEFAULT, "default column value")); options.addOption(createSimpleOption("o", OWNER, "display entity owner")); -options.addOption(createArgOption("r", ROLE, "role name")); options.addOption(createArgOption(COLUMNFILE, "CSV file describing columns")); // Properties and tags can have multiple values Review Comment: hi justin, the comments are changed as follows, Is that right?  -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5815] fix(web): config oceanbase column types [gravitino]
mchades merged PR #5816: URL: https://github.com/apache/gravitino/pull/5816 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5602] feat(core): Add model storage schema layout (part-2) [gravitino]
xloya commented on code in PR #5728: URL: https://github.com/apache/gravitino/pull/5728#discussion_r1877941961 ## core/src/main/java/org/apache/gravitino/storage/relational/service/ModelVersionMetaService.java: ## @@ -0,0 +1,288 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.storage.relational.service; + +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.Multimap; +import java.io.IOException; +import java.util.Collections; +import java.util.List; +import java.util.Locale; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; +import org.apache.gravitino.Entity; +import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.Namespace; +import org.apache.gravitino.exceptions.NoSuchEntityException; +import org.apache.gravitino.meta.ModelEntity; +import org.apache.gravitino.meta.ModelVersionEntity; +import org.apache.gravitino.storage.relational.mapper.ModelMetaMapper; +import org.apache.gravitino.storage.relational.mapper.ModelVersionAliasRelMapper; +import org.apache.gravitino.storage.relational.mapper.ModelVersionMetaMapper; +import org.apache.gravitino.storage.relational.po.ModelPO; +import org.apache.gravitino.storage.relational.po.ModelVersionAliasRelPO; +import org.apache.gravitino.storage.relational.po.ModelVersionPO; +import org.apache.gravitino.storage.relational.utils.ExceptionUtils; +import org.apache.gravitino.storage.relational.utils.POConverters; +import org.apache.gravitino.storage.relational.utils.SessionUtils; +import org.apache.gravitino.utils.NameIdentifierUtil; +import org.apache.gravitino.utils.NamespaceUtil; +import org.glassfish.jersey.internal.guava.Lists; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class ModelVersionMetaService { + + private static final Logger LOG = LoggerFactory.getLogger(ModelVersionMetaService.class); + + private static final ModelVersionMetaService INSTANCE = new ModelVersionMetaService(); + + public static ModelVersionMetaService getInstance() { +return INSTANCE; + } + + private ModelVersionMetaService() {} + + public List listModelVersionsByNamespace(Namespace ns) { +NamespaceUtil.checkModelVersion(ns); + +NameIdentifier modelIdent = NameIdentifier.of(ns.levels()); +// Will throw a NoSuchEntityException if the model does not exist. +ModelEntity modelEntity = ModelMetaService.getInstance().getModelByIdentifier(modelIdent); + +List modelVersionPOs = +SessionUtils.getWithoutCommit( +ModelVersionMetaMapper.class, +mapper -> mapper.listModelVersionMetasByModelId(modelEntity.id())); + +if (modelVersionPOs.isEmpty()) { + return Collections.emptyList(); +} + +// Get the aliases for all the model versions. +List aliasRelPOs = +SessionUtils.getWithoutCommit( +ModelVersionAliasRelMapper.class, +mapper -> mapper.selectModelVersionAliasRelByModelId(modelEntity.id())); +Multimap aliasRelPOsByModelVersion = +ArrayListMultimap.create(); +aliasRelPOs.forEach(r -> aliasRelPOsByModelVersion.put(r.getModelVersion(), r)); + +return modelVersionPOs.stream() +.map( +m -> { + List versionAliasRelPOs = + Lists.newArrayList(aliasRelPOsByModelVersion.get(m.getModelVersion())); + return POConverters.fromModelVersionPO(modelIdent, m, versionAliasRelPOs); +}) +.collect(Collectors.toList()); + } + + public ModelVersionEntity getModelVersionByIdentifier(NameIdentifier ident) { +NameIdentifierUtil.checkModelVersion(ident); + +NameIdentifier modelIdent = NameIdentifier.of(ident.namespace().levels()); +// Will throw a NoSuchEntityException if the model does not exist. +ModelEntity modelEntity = ModelMetaService.getInstance().getModelByIdentifier(modelIdent); + +Integer modelVersion; +try { + modelVersion = Integer.valueOf(ident.name()); +} catch (NumberFormatException e) { + modelVersion = null; +} + +final Integer copiedModelVersion = modelVersion; +ModelVersionPO modelVersionPO = +SessionUtils
Re: [I] [Improvement] OB-Catalog(WebUI): missing column data types [gravitino]
mchades closed issue #5815: [Improvement] OB-Catalog(WebUI): missing column data types URL: https://github.com/apache/gravitino/issues/5815 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Improvement] OB-Catalog(WebUI): missing column data types [gravitino]
mchades closed issue #5815: [Improvement] OB-Catalog(WebUI): missing column data types URL: https://github.com/apache/gravitino/issues/5815 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
(gravitino) branch main updated: [#5815] fix(web): config oceanbase column types (#5816)
This is an automated email from the ASF dual-hosted git repository. mchades pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/gravitino.git The following commit(s) were added to refs/heads/main by this push: new 8fa24a475 [#5815] fix(web): config oceanbase column types (#5816) 8fa24a475 is described below commit 8fa24a475904b5ab39891126a08c24a30b49ee7e Author: Qian Xia AuthorDate: Tue Dec 10 19:39:36 2024 +0800 [#5815] fix(web): config oceanbase column types (#5816) ### What changes were proposed in this pull request? config oceanbase column types ### Why are the changes needed? N/A Fix: #5815 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? manually --- web/web/src/lib/utils/initial.js | 20 1 file changed, 20 insertions(+) diff --git a/web/web/src/lib/utils/initial.js b/web/web/src/lib/utils/initial.js index f7328205d..6d6efe84b 100644 --- a/web/web/src/lib/utils/initial.js +++ b/web/web/src/lib/utils/initial.js @@ -565,6 +565,26 @@ const relationalColumnTypeMap = { 'timestamp', 'timestamp_tz', 'varchar' + ], + 'jdbc-oceanbase': [ +'binary', +'byte', +'byte unsigned', +'char', +'date', +'decimal', +'double', +'float', +'integer', +'integer unsigned', +'long', +'long unsigned', +'short', +'short unsigned', +'string', +'time', +'timestamp', +'varchar' ] }
Re: [I] [Subtask] [flink-connector] Support iceberg catalog [gravitino]
sunxiaojian commented on issue #3515: URL: https://github.com/apache/gravitino/issues/3515#issuecomment-2531311183 @coolderli @xunliu please assign it to me, thanks -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] [MINOR] docs: fix typo in catalog.management exception message [gravitino]
predator4ann opened a new pull request, #5812: URL: https://github.com/apache/gravitino/pull/5812 ### What changes were proposed in this pull request? The exception message should be `catalog.management = dynamic` when the trino configuration is incorrect. ### Why are the changes needed? (Please clarify why the changes are needed. For instance, 1. If you propose a new API, clarify the use case for a new API. 2. If you fix a bug, describe the bug.) Fix: # (issue) ### Does this PR introduce _any_ user-facing change? (Please list the user-facing changes introduced by your change, including 1. Change in user-facing APIs. 2. Addition or removal of property keys.) ### How was this patch tested? (Please test your changes, and provide instructions on how to test it: 1. If you add a feature or fix a bug, add a test to cover your changes. 2. If you fix a flaky test, repeat it for many times to prove it works.) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5760][#5780] fix(catalog): fix drop catalog error [gravitino]
mchades commented on code in PR #5761: URL: https://github.com/apache/gravitino/pull/5761#discussion_r1877646142 ## core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java: ## @@ -677,11 +678,70 @@ public boolean dropCatalog(NameIdentifier ident, boolean force) } catch (NoSuchMetalakeException | NoSuchCatalogException ignored) { return false; -} catch (IOException e) { +} catch (GravitinoRuntimeException e) { + throw e; + +} catch (Exception e) { throw new RuntimeException(e); } } + /** + * Check if the given list of schema entities contains any currently existing user-created + * schemas. + * + * This method determines if there are valid user-created schemas by comparing the provided + * schema entities with the actual schemas currently existing in the external catalog. It + * excludes: + * + * + * 1. Automatically generated schemas (such as Kafka catalog's "default" schema or + * JDBC-PostgreSQL catalog's "public" schema). + * 2. Schemas that have been dropped externally but still exist in the entity store. + * + * + * @param schemaEntities The list of schema entities to check. + * @param catalogEntity The catalog entity to which the schemas belong. + * @param catalogWrapper The catalog wrapper for the catalog. + * @return True if the list of schema entities contains any valid user-created schemas, false + * otherwise. + * @throws Exception If an error occurs while checking the schemas. + */ + private boolean containsUserCreatedSchemas( Review Comment: Basically all schemas are created by users, except: - "public" schema in PG is created by PG itself, not a user-created schema - "default" schema in kafka catalog is created by Kafka catalog itself, not a user-created schema This is unrelated to whether it was created by Gravitino, but rather pertains to whether it was created by a user. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Bug report] http client out of time [gravitino]
an-shi-chi-fan commented on issue #5765: URL: https://github.com/apache/gravitino/issues/5765#issuecomment-2530879552 thanks @mchades @FANNG1 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Bug report] http client out of time [gravitino]
an-shi-chi-fan commented on issue #5765: URL: https://github.com/apache/gravitino/issues/5765#issuecomment-2530882791 hive metastore service time out -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Bug report] http client out of time [gravitino]
an-shi-chi-fan closed issue #5765: [Bug report] http client out of time URL: https://github.com/apache/gravitino/issues/5765 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5623] feat(python): supports credential API in python client [gravitino]
xloya commented on code in PR #5777: URL: https://github.com/apache/gravitino/pull/5777#discussion_r1877973226 ## clients/client-python/gravitino/dto/responses/credential_response.py: ## @@ -0,0 +1,44 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from typing import List +from dataclasses import dataclass, field +from dataclasses_json import config + +from gravitino.dto.credential_dto import CredentialDTO +from gravitino.dto.responses.base_response import BaseResponse +from gravitino.exceptions.base import IllegalArgumentException + + +@dataclass +class CredentialResponse(BaseResponse): +"""Response for credential response.""" + +_credentials: List[CredentialDTO] = field(metadata=config(field_name="credentials")) + +def credentials(self) -> List[CredentialDTO]: +return self._credentials + +def validate(self): +"""Validates the response data. + +Raises: +IllegalArgumentException if credentials are None. +""" +if self._credentials is None: Review Comment: It's better to check the length of `List` object too. ## clients/client-python/gravitino/client/metadata_object_credential_operations.py: ## @@ -0,0 +1,67 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import logging +from typing import List +from gravitino.api.credential.supports_credentials import SupportsCredentials +from gravitino.api.credential.credential import Credential +from gravitino.api.metadata_object import MetadataObject +from gravitino.dto.credential_dto import CredentialDTO +from gravitino.dto.responses.credential_response import CredentialResponse +from gravitino.exceptions.handlers.credential_error_handler import ( +CREDENTIAL_ERROR_HANDLER, +) +from gravitino.utils import HTTPClient +from gravitino.utils.credential_utils import CredentialUtils + +logger = logging.getLogger(__name__) + + +class MetadataObjectCredentialOperations(SupportsCredentials): +def __init__( +self, +metalake_name: str, +metadata_object: MetadataObject, +rest_client: HTTPClient, +): +self._rest_client = rest_client +t = metadata_object.type().value Review Comment: It's better to make this var more meaningful. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5734] feat (gvfs-fuse): Gvfs-fuse basic FUSE-level implementation and code structure layout [gravitino]
xunliu commented on code in PR #5738: URL: https://github.com/apache/gravitino/pull/5738#discussion_r1878030592 ## clients/filesystem-fuse/src/fuse_api_handle.rs: ## @@ -0,0 +1,437 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +use crate::filesystem::{FileStat, FileSystemContext, RawFileSystem, SimpleFileSystem}; +use fuse3::path::prelude::{ReplyData, ReplyOpen, ReplyStatFs, ReplyWrite}; +use fuse3::path::Request; +use fuse3::raw::prelude::{ +FileAttr, ReplyAttr, ReplyCreated, ReplyDirectory, ReplyDirectoryPlus, ReplyEntry, ReplyInit, +}; +use fuse3::raw::reply::{DirectoryEntry, DirectoryEntryPlus}; +use fuse3::raw::Filesystem; +use fuse3::FileType::{Directory, RegularFile}; +use fuse3::{Errno, FileType, Inode, SetAttr, Timestamp}; +use futures_util::stream::BoxStream; +use futures_util::StreamExt; +use futures_util::{stream, FutureExt}; +use std::ffi::{OsStr, OsString}; +use std::num::NonZeroU32; +use std::time::{Duration, SystemTime}; + +pub(crate) struct FuseApiHandle { +local_fs: T, +default_ttl: Duration, +fs_context: FileSystemContext, +} + +impl FuseApiHandle { +pub fn new(fs: T, context: FileSystemContext) -> Self { +FuseApiHandle { +local_fs: fs, +default_ttl: Duration::from_secs(1), +fs_context: context, +} +} + +pub async fn get_file_path(&self, inode: u64) -> String { +self.local_fs.get_file_path(inode).await +} + +async fn get_modified_file_stat( +&self, +inode: u64, +size: Option, +atime: Option, +mtime: Option, +) -> Result { +let file_stat = self.local_fs.stat(inode).await?; +let mut nf = FileStat::clone(&file_stat); +size.map(|size| { +nf.size = size; +}); +atime.map(|atime| { +nf.atime = atime; +}); +mtime.map(|mtime| { +nf.mtime = mtime; +}); +Ok(nf) +} +} + +impl Filesystem for FuseApiHandle { +async fn init(&self, _req: Request) -> fuse3::Result { +self.local_fs.init().await; +Ok(ReplyInit { +max_write: NonZeroU32::new(16 * 1024).unwrap(), +}) +} + +async fn destroy(&self, _req: Request) {} + +async fn lookup( +&self, +_req: Request, +parent: Inode, +name: &OsStr, +) -> fuse3::Result { +let file_stat = self.local_fs.lookup(parent, name.to_str().unwrap()).await?; +Ok(ReplyEntry { +ttl: self.default_ttl, +attr: fstat_to_file_attr(&file_stat, &self.fs_context), +generation: 0, +}) +} + +async fn getattr( +&self, +_req: Request, +inode: Inode, +fh: Option, +_flags: u32, +) -> fuse3::Result { +// check the opened file inode is the same as the inode +if let Some(fh) = fh { +self.local_fs.valid_file_id(inode, fh).await?; +} + +let file_stat = self.local_fs.stat(inode).await?; +Ok(ReplyAttr { +ttl: self.default_ttl, +attr: fstat_to_file_attr(&file_stat, &self.fs_context), +}) +} + +async fn setattr( Review Comment: Please change function name to `set_attr` ## clients/filesystem-fuse/src/fuse_api_handle.rs: ## @@ -0,0 +1,437 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +use cra
[PR] [#5192] [Feature] Support Catalog Operation DDL for paimon-catalog [gravitino]
hdygxsj opened a new pull request, #5818: URL: https://github.com/apache/gravitino/pull/5818 ### What changes were proposed in this pull request? Support Catalog Operation DDL for paimon-catalog ### Why are the changes needed? close #5192 ### Does this PR introduce _any_ user-facing change? None ### How was this patch tested? org.apache.gravitino.flink.connector.paimon.TestPaimonPropertiesConverter org.apache.gravitino.flink.connector.integration.test.paimon.FlinkPaimonCatalogIT -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [Improvement] Remove `RocksDBKvBackend` and related code & dependencies from code base. [gravitino]
yuqi1129 closed issue #5633: [Improvement] Remove `RocksDBKvBackend` and related code & dependencies from code base. URL: https://github.com/apache/gravitino/issues/5633 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5633] remove the key-value storage backend logic [gravitino]
yuqi1129 merged PR #5645: URL: https://github.com/apache/gravitino/pull/5645 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [#5734] feat (gvfs-fuse): Gvfs-fuse basic FUSE-level implementation and code structure layout [gravitino]
diqiu50 commented on code in PR #5738: URL: https://github.com/apache/gravitino/pull/5738#discussion_r1878050265 ## clients/filesystem-fuse/src/fuse_api_handle.rs: ## @@ -0,0 +1,437 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +use crate::filesystem::{FileStat, FileSystemContext, RawFileSystem, SimpleFileSystem}; +use fuse3::path::prelude::{ReplyData, ReplyOpen, ReplyStatFs, ReplyWrite}; +use fuse3::path::Request; +use fuse3::raw::prelude::{ +FileAttr, ReplyAttr, ReplyCreated, ReplyDirectory, ReplyDirectoryPlus, ReplyEntry, ReplyInit, +}; +use fuse3::raw::reply::{DirectoryEntry, DirectoryEntryPlus}; +use fuse3::raw::Filesystem; +use fuse3::FileType::{Directory, RegularFile}; +use fuse3::{Errno, FileType, Inode, SetAttr, Timestamp}; +use futures_util::stream::BoxStream; +use futures_util::StreamExt; +use futures_util::{stream, FutureExt}; +use std::ffi::{OsStr, OsString}; +use std::num::NonZeroU32; +use std::time::{Duration, SystemTime}; + +pub(crate) struct FuseApiHandle { +local_fs: T, +default_ttl: Duration, +fs_context: FileSystemContext, +} + +impl FuseApiHandle { +pub fn new(fs: T, context: FileSystemContext) -> Self { +FuseApiHandle { +local_fs: fs, +default_ttl: Duration::from_secs(1), +fs_context: context, +} +} + +pub async fn get_file_path(&self, inode: u64) -> String { +self.local_fs.get_file_path(inode).await +} + +async fn get_modified_file_stat( +&self, +inode: u64, +size: Option, +atime: Option, +mtime: Option, +) -> Result { +let file_stat = self.local_fs.stat(inode).await?; +let mut nf = FileStat::clone(&file_stat); +size.map(|size| { +nf.size = size; +}); +atime.map(|atime| { +nf.atime = atime; +}); +mtime.map(|mtime| { +nf.mtime = mtime; +}); +Ok(nf) +} +} + +impl Filesystem for FuseApiHandle { +async fn init(&self, _req: Request) -> fuse3::Result { +self.local_fs.init().await; +Ok(ReplyInit { +max_write: NonZeroU32::new(16 * 1024).unwrap(), +}) +} + +async fn destroy(&self, _req: Request) {} + +async fn lookup( +&self, +_req: Request, +parent: Inode, +name: &OsStr, +) -> fuse3::Result { +let file_stat = self.local_fs.lookup(parent, name.to_str().unwrap()).await?; +Ok(ReplyEntry { +ttl: self.default_ttl, +attr: fstat_to_file_attr(&file_stat, &self.fs_context), +generation: 0, +}) +} + +async fn getattr( +&self, +_req: Request, +inode: Inode, +fh: Option, +_flags: u32, +) -> fuse3::Result { +// check the opened file inode is the same as the inode +if let Some(fh) = fh { +self.local_fs.valid_file_id(inode, fh).await?; +} + +let file_stat = self.local_fs.stat(inode).await?; +Ok(ReplyAttr { +ttl: self.default_ttl, +attr: fstat_to_file_attr(&file_stat, &self.fs_context), +}) +} + +async fn setattr( +&self, +req: Request, +inode: Inode, +fh: Option, +set_attr: SetAttr, +) -> fuse3::Result { +let new_file_stat = self +.get_modified_file_stat(inode, set_attr.size, set_attr.atime, set_attr.mtime) +.await?; +let attr = fstat_to_file_attr(&new_file_stat, &self.fs_context); +self.local_fs.set_attr(inode, &new_file_stat).await?; +Ok(ReplyAttr { +ttl: self.default_ttl, +attr: attr, +}) +} + +async fn mkdir( +&self, +req: Request, +parent: Inode, +name: &OsStr, +mode: u32, +umask: u32, +) -> fuse3::Result { +let handle_id = self +.local_fs +.create_dir(parent, name.to_str().unwrap()) +.await?; +Ok(ReplyEntry { +ttl: self.default_ttl, +attr: dummy_file_attr( +handle_id.file_id, +Dir