Re: [PR] [#4953] feat(hudi-catalog): add Hudi catalog IT and enable module [gravitino]
jerryshao merged PR #4965: URL: https://github.com/apache/gravitino/pull/4965 -- 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 IT and enable module for Hudi catalog [gravitino]
jerryshao closed issue #4953: [Subtask] add IT and enable module for Hudi catalog URL: https://github.com/apache/gravitino/issues/4953 -- 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: [#4953] feat(hudi-catalog): add Hudi catalog IT and enable module (#4965)
This is an automated email from the ASF dual-hosted git repository.
jshao 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 cc277566b [#4953] feat(hudi-catalog): add Hudi catalog IT and enable
module (#4965)
cc277566b is described below
commit cc277566bcf8251786fc5e18424b9a0ca5354849
Author: mchades
AuthorDate: Sat Oct 12 18:58:17 2024 +0800
[#4953] feat(hudi-catalog): add Hudi catalog IT and enable module (#4965)
### What changes were proposed in this pull request?
add Hudi catalog IT and enable module
### Why are the changes needed?
Fix: #4953
### Does this PR introduce _any_ user-facing change?
yes. The Hudi catalog is available now
### How was this patch tested?
ITs added
---
build.gradle.kts | 1 +
catalogs/catalog-hive/build.gradle.kts | 2 +-
catalogs/catalog-kafka/build.gradle.kts| 9 -
catalogs/catalog-lakehouse-hudi/build.gradle.kts | 136 --
.../lakehouse/hudi/HudiCatalogOperations.java | 3 +-
.../src/main/resources/hive-site.xml.template | 21 +
.../src/main/resources/lakehouse-hudi.conf | 22 +
.../lakehouse/hudi/TestHudiCatalogOperations.java | 4 +-
.../hudi/integration/test/HudiCatalogHMSIT.java| 503 +
.../src/test/resources/log4j2.properties | 73 +++
catalogs/hive-metastore-common/build.gradle.kts| 2 +-
.../apache/gravitino/dto/util/DTOConverters.java | 4 +-
flink-connector/flink/build.gradle.kts | 2 +-
gradle/libs.versions.toml | 1 +
.../src/test/resources/log4j2.properties | 73 +++
.../apache/gravitino/server/GravitinoServer.java | 2 +
spark-connector/spark-common/build.gradle.kts | 2 +-
spark-connector/v3.3/spark/build.gradle.kts| 2 +-
spark-connector/v3.4/spark/build.gradle.kts| 2 +-
spark-connector/v3.5/spark/build.gradle.kts| 2 +-
20 files changed, 813 insertions(+), 53 deletions(-)
diff --git a/build.gradle.kts b/build.gradle.kts
index 75d0d8759..6db5f00cc 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -780,6 +780,7 @@ tasks {
":catalogs:catalog-hive:copyLibAndConfig",
":catalogs:catalog-lakehouse-iceberg:copyLibAndConfig",
":catalogs:catalog-lakehouse-paimon:copyLibAndConfig",
+ "catalogs:catalog-lakehouse-hudi:copyLibAndConfig",
":catalogs:catalog-jdbc-doris:copyLibAndConfig",
":catalogs:catalog-jdbc-mysql:copyLibAndConfig",
":catalogs:catalog-jdbc-postgresql:copyLibAndConfig",
diff --git a/catalogs/catalog-hive/build.gradle.kts
b/catalogs/catalog-hive/build.gradle.kts
index 2afb48f9a..aca8959df 100644
--- a/catalogs/catalog-hive/build.gradle.kts
+++ b/catalogs/catalog-hive/build.gradle.kts
@@ -58,7 +58,7 @@ dependencies {
exclude("com.google.code.findbugs", "sr305")
exclude("com.tdunning", "json")
exclude("com.zaxxer", "HikariCP")
-exclude("io.dropwizard.metricss")
+exclude("io.dropwizard.metrics")
exclude("javax.transaction", "transaction-api")
exclude("org.apache.ant")
exclude("org.apache.avro")
diff --git a/catalogs/catalog-kafka/build.gradle.kts
b/catalogs/catalog-kafka/build.gradle.kts
index fe8e6086f..6a8c104c7 100644
--- a/catalogs/catalog-kafka/build.gradle.kts
+++ b/catalogs/catalog-kafka/build.gradle.kts
@@ -97,15 +97,6 @@
tasks.getByName("generateMetadataFileForMavenJavaPublication") {
}
tasks.test {
- doFirst {
-val testMode = project.properties["testMode"] as? String ?: "embedded"
-if (testMode == "deploy") {
- environment("GRAVITINO_HOME", project.rootDir.path +
"/distribution/package")
-} else if (testMode == "embedded") {
- environment("GRAVITINO_HOME", project.rootDir.path)
-}
- }
-
val skipITs = project.hasProperty("skipITs")
if (skipITs) {
// Exclude integration tests
diff --git a/catalogs/catalog-lakehouse-hudi/build.gradle.kts
b/catalogs/catalog-lakehouse-hudi/build.gradle.kts
index eef90f029..814965ec0 100644
--- a/catalogs/catalog-lakehouse-hudi/build.gradle.kts
+++ b/catalogs/catalog-lakehouse-hudi/build.gradle.kts
@@ -27,55 +27,38 @@ plugins {
val scalaVersion: String = project.properties["scalaVersion"] as? String ?:
extra["defaultScalaVersion"].toString()
val fullSparkVersion: String = libs.versions.spark34.get()
val sparkVersion = fullSparkVersion.split(".").take(2).joinToString(".")
+val hudiVersion = libs.versions.hudi.get()
dependencies {
implementation(project(":api")) {
-exclude(group = "*")
+exclude("*")
}
implementation(project(":common")) {
-exclude(group = "*")
+exclude("*")
}
implementation(project(":catalogs:hive-metastore-common"))
implementation(project(":core")) {
-exclude(group = "*")
+exclude("*")
}
Re: [PR] [#4867] feat(core): Add storage support for columns in Gravitino [gravitino]
jerryshao commented on PR #5078: URL: https://github.com/apache/gravitino/pull/5078#issuecomment-2408521985 > > I feel like it is not that easy to validate if the column is deleted or not through entity store interface, because we columns cannot managed directly through this interface, it is handled when we do the table operations. > > I see. Maybe we can refer to the `TestJDBCBackend.listFilesetVersions()` method, query the data through JDBC connection directly. OK, let me check 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] Execution flow of drop a role [gravitino]
jerqi closed issue #4509: [Improvement] Execution flow of drop a role URL: https://github.com/apache/gravitino/issues/4509 -- 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] Incorrect Parsing of array<> Data Type in Database Metadata [gravitino]
lsyulong commented on issue #5098: URL: https://github.com/apache/gravitino/issues/5098#issuecomment-2408850954 Hello, has anyone submitted a PR to fix this issue? I would like to give it a try. Can you assign it to me? If you encounter any problems, feel free to communicate with you at any time -- 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] [#4953] feat(hudi-catalog): add Hudi catalog IT and enable module [gravitino]
mchades commented on code in PR #4965:
URL: https://github.com/apache/gravitino/pull/4965#discussion_r1797622083
##
catalogs/catalog-lakehouse-hudi/build.gradle.kts:
##
@@ -115,12 +110,29 @@ dependencies {
testImplementation(libs.hadoop2.auth) {
exclude("*")
}
+ testImplementation(libs.hadoop2.hdfs)
testImplementation(libs.hadoop2.mapreduce.client.core) {
exclude("*")
}
testImplementation(libs.htrace.core4)
testImplementation(libs.junit.jupiter.api)
- testImplementation(libs.woodstox.core)
+ testImplementation(libs.mysql.driver)
+ testImplementation(libs.postgresql.driver)
+ testImplementation(libs.prometheus.dropwizard)
Review Comment:
`libs.mysql.driver` and `libs.postgresql.driver` are for Gravitino different
backends IT.
For `libs.prometheus.dropwizard`, if I removed it, will encountered the
below error:
```
Caused by: java.lang.NoSuchMethodError:
io.prometheus.client.dropwizard.DropwizardExports.(Lcom/codahale/metrics/MetricRegistry;Lio/prometheus/client/dropwizard/samplebuilder/SampleBuilder;)V
at
org.apache.gravitino.metrics.MetricsSystem.registerMetricsToPrometheusRegistry(MetricsSystem.java:186)
at
org.apache.gravitino.metrics.MetricsSystem.start(MetricsSystem.java:121)
at org.apache.gravitino.GravitinoEnv.start(GravitinoEnv.java:312)
at
org.apache.gravitino.server.GravitinoServer.start(GravitinoServer.java:147)
at
org.apache.gravitino.server.GravitinoServer.main(GravitinoServer.java:169)
at
org.apache.gravitino.integration.test.MiniGravitino.lambda$start$0(MiniGravitino.java:150)
```
Because the `hudi_spark_bundle` jar includes the same class
`DropwizardExports` which is conflict with the Gravitino's dependency so I have
to declare the `libs.prometheus.dropwizard` dependency explicitly
--
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] [#4951] improvement(test): Reduce fields shares between different IT to make AbstractIT more independent. [gravitino]
yuqi1129 commented on code in PR #4996:
URL: https://github.com/apache/gravitino/pull/4996#discussion_r1797624956
##
integration-test-common/src/test/java/org/apache/gravitino/integration/test/util/AbstractIT.java:
##
@@ -58,61 +58,61 @@
import org.apache.gravitino.server.web.JettyServerConfig;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.extension.ExtendWith;
-import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.CsvSource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.testcontainers.shaded.org.awaitility.Awaitility;
@ExtendWith({PrintFuncNameExtension.class, CloseContainerExtension.class})
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
Review Comment:
Yes. Please see `SparkHiveCatalogIT33` and `SparkHiveCatalogIT`
--
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] [#4951] improvement(test): Reduce fields shares between different IT to make AbstractIT more independent. [gravitino]
yuqi1129 commented on code in PR #4996:
URL: https://github.com/apache/gravitino/pull/4996#discussion_r1797625045
##
integration-test-common/src/test/java/org/apache/gravitino/integration/test/util/AbstractIT.java:
##
@@ -248,15 +248,8 @@ public static String startAndInitMySQLBackend() {
}
}
- @ParameterizedTest
- @CsvSource({
-"embedded, jdbcBackend",
-"embedded, kvBackend",
-"deploy, jdbcBackend",
-"deploy, kvBackend"
- })
Review Comment:
It's pointless and won't actually happen.
--
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] [#5057] Added first part of CLI code [gravitino]
justinmclean commented on code in PR #5058:
URL: https://github.com/apache/gravitino/pull/5058#discussion_r1797720606
##
clients/cli/src/main/java/org/apache/gravitino/cli/FullName.java:
##
@@ -0,0 +1,123 @@
+/*
+ * 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.cli;
+
+import org.apache.commons.cli.CommandLine;
+
+/**
+ * Extracts different arts of a full name (dot seperated) from the command
line input. This includes
+ * metalake, catalog, schema, and table names.
+ */
+public class FullName {
+ CommandLine line;
+
+ /**
+ * Constructor for the {@code FullName} class.
+ *
+ * @param line The parsed command line arguments.
+ */
+ public FullName(CommandLine line) {
+this.line = line;
+ }
+
+ /**
+ * Retrieves the metalake name from the command line options, environment
variables, or the first
+ * part of the full name.
+ *
+ * @return The metalake name, or null if not found.
+ */
+ public String getMetalakeName() {
+String metalakeEnv = System.getenv("GRAVITINO_METALAKE");
+
+// Check if the metalake name is specified as a command line option
+if (line.hasOption(GravitinoOptions.METALAKE)) {
+ return line.getOptionValue(GravitinoOptions.METALAKE);
+ // Check if the metalake name is set as an environment variable
+} else if (metalakeEnv != null) {
+ return metalakeEnv;
+ // Extract the metalake name from the full name option
+} else if (line.hasOption(GravitinoOptions.NAME)) {
+ return line.getOptionValue(GravitinoOptions.NAME).split("\\.")[0];
+}
+
+return null;
Review Comment:
It's not needed here as the CLI API detects that earlier on.
--
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] [#5057] Added first part of CLI code [gravitino]
justinmclean commented on code in PR #5058:
URL: https://github.com/apache/gravitino/pull/5058#discussion_r1797720541
##
clients/cli/build.gradle.kts:
##
@@ -0,0 +1,59 @@
+/*
+ * 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.
+ */
+plugins {
+ `maven-publish`
+ id("java")
+ id("idea")
+}
+
+dependencies {
+ implementation(project(":clients:client-java"))
+ implementation(project(":api"))
+ implementation(project(":common"))
+ implementation("commons-cli:commons-cli:1.9.0")
+ implementation("org.slf4j:slf4j-api:2.0.16")
+ implementation("org.slf4j:slf4j-simple:2.0.16")
+
+ testImplementation(libs.junit.jupiter.api)
+ testImplementation(libs.junit.jupiter.params)
+ testImplementation(libs.mockito.core)
+
+ testRuntimeOnly(libs.junit.jupiter.engine)
+}
+
+tasks.build {
+ dependsOn("javadoc")
Review Comment:
So don't generate java docs? Javadocs are still useful for developers who
are working on the code.
--
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] README.md should refer the link to our doc website but not doc's source directory [gravitino]
yuqi1129 commented on issue #5109: URL: https://github.com/apache/gravitino/issues/5109#issuecomment-2408772436 > Hi, > > > > I'm interested on working on this. Could I please be assigned? Thanks. Sure,please go ahead. -- 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] [#5057] Added first part of CLI code [gravitino]
justinmclean commented on code in PR #5058:
URL: https://github.com/apache/gravitino/pull/5058#discussion_r1797720606
##
clients/cli/src/main/java/org/apache/gravitino/cli/FullName.java:
##
@@ -0,0 +1,123 @@
+/*
+ * 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.cli;
+
+import org.apache.commons.cli.CommandLine;
+
+/**
+ * Extracts different arts of a full name (dot seperated) from the command
line input. This includes
+ * metalake, catalog, schema, and table names.
+ */
+public class FullName {
+ CommandLine line;
+
+ /**
+ * Constructor for the {@code FullName} class.
+ *
+ * @param line The parsed command line arguments.
+ */
+ public FullName(CommandLine line) {
+this.line = line;
+ }
+
+ /**
+ * Retrieves the metalake name from the command line options, environment
variables, or the first
+ * part of the full name.
+ *
+ * @return The metalake name, or null if not found.
+ */
+ public String getMetalakeName() {
+String metalakeEnv = System.getenv("GRAVITINO_METALAKE");
+
+// Check if the metalake name is specified as a command line option
+if (line.hasOption(GravitinoOptions.METALAKE)) {
+ return line.getOptionValue(GravitinoOptions.METALAKE);
+ // Check if the metalake name is set as an environment variable
+} else if (metalakeEnv != null) {
+ return metalakeEnv;
+ // Extract the metalake name from the full name option
+} else if (line.hasOption(GravitinoOptions.NAME)) {
+ return line.getOptionValue(GravitinoOptions.NAME).split("\\.")[0];
+}
+
+return null;
Review Comment:
It's unnecessary here as the CLI API detects that earlier (when parsing the
command line). Also, the code handles the case when this returns nulls,
--
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] [#5057] Added first part of CLI code [gravitino]
justinmclean commented on code in PR #5058:
URL: https://github.com/apache/gravitino/pull/5058#discussion_r1797720606
##
clients/cli/src/main/java/org/apache/gravitino/cli/FullName.java:
##
@@ -0,0 +1,123 @@
+/*
+ * 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.cli;
+
+import org.apache.commons.cli.CommandLine;
+
+/**
+ * Extracts different arts of a full name (dot seperated) from the command
line input. This includes
+ * metalake, catalog, schema, and table names.
+ */
+public class FullName {
+ CommandLine line;
+
+ /**
+ * Constructor for the {@code FullName} class.
+ *
+ * @param line The parsed command line arguments.
+ */
+ public FullName(CommandLine line) {
+this.line = line;
+ }
+
+ /**
+ * Retrieves the metalake name from the command line options, environment
variables, or the first
+ * part of the full name.
+ *
+ * @return The metalake name, or null if not found.
+ */
+ public String getMetalakeName() {
+String metalakeEnv = System.getenv("GRAVITINO_METALAKE");
+
+// Check if the metalake name is specified as a command line option
+if (line.hasOption(GravitinoOptions.METALAKE)) {
+ return line.getOptionValue(GravitinoOptions.METALAKE);
+ // Check if the metalake name is set as an environment variable
+} else if (metalakeEnv != null) {
+ return metalakeEnv;
+ // Extract the metalake name from the full name option
+} else if (line.hasOption(GravitinoOptions.NAME)) {
+ return line.getOptionValue(GravitinoOptions.NAME).split("\\.")[0];
+}
+
+return null;
Review Comment:
It's unnecessary here as the CLI API detects that earlier (when parsing the
command line). Also, the code handles the case when this returns nulls.
--
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] README.md should refer the link to our doc website but not doc's source directory [gravitino]
SeanAverS commented on issue #5109: URL: https://github.com/apache/gravitino/issues/5109#issuecomment-2408680452 Hi, I'm interested on working on this. Could I please be assigned? 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: [PR] [#5071] Improvement (test): Add integration tests for Trino cascading queries [gravitino]
diqiu50 commented on code in PR #5073:
URL: https://github.com/apache/gravitino/pull/5073#discussion_r1797626917
##
.github/workflows/trino-integration-test.yml:
##
@@ -76,7 +76,7 @@ jobs:
- name: Package Gravitino
run: |
- ./gradlew compileDistribution -x test -PjdkVersion=${{
matrix.java-version }}
+ ./gradlew build compileDistribution -x test -PjdkVersion=${{
matrix.java-version }}
Review Comment:
the `compileDistribution` task does not build the Trino connector
--
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] [#5071] Improvement (test): Add integration tests for Trino cascading queries [gravitino]
diqiu50 commented on code in PR #5073: URL: https://github.com/apache/gravitino/pull/5073#discussion_r1797615808 ## trino-connector/integration-test/trino-test-tools/trino-cascading-env/init/trino-local/config/catalog/trino.properties: ## @@ -0,0 +1,21 @@ +# +# 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. + +connector.name=trino +connection-url=jdbc:trino://trino-remote:8080/tpcds Review Comment: That URL is a valid connection-url used by the Docker Compose environment. -- 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] [#4867] feat(core): Add storage support for columns in Gravitino [gravitino]
xloya commented on PR #5078: URL: https://github.com/apache/gravitino/pull/5078#issuecomment-2408458762 > I feel like it is not that easy to validate if the column is deleted or not through entity store interface, because we columns cannot managed directly through this interface, it is handled when we do the table operations. I see. Maybe we can refer to the `TestJDBCBackend.listFilesetVersions()` method, query the data through JDBC connection directly. -- 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] [#3920] feat(catalog-lakehouse-paimon): Support jdbc backend for Paimon Catalog [gravitino]
caican00 commented on code in PR #5087: URL: https://github.com/apache/gravitino/pull/5087#discussion_r1797634852 ## catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonCatalogPropertiesMetadata.java: ## @@ -46,14 +46,26 @@ public class PaimonCatalogPropertiesMetadata extends BaseCatalogPropertiesMetada public static final String PAIMON_METASTORE = "metastore"; public static final String WAREHOUSE = "warehouse"; public static final String URI = "uri"; + public static final String JDBC_USER = "jdbc.user"; Review Comment: Paimon use `jdbc.` as property prefix. -- 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] [#3920] feat(catalog-lakehouse-paimon): Support jdbc backend for Paimon Catalog [gravitino]
caican00 commented on code in PR #5087: URL: https://github.com/apache/gravitino/pull/5087#discussion_r1797634852 ## catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonCatalogPropertiesMetadata.java: ## @@ -46,14 +46,26 @@ public class PaimonCatalogPropertiesMetadata extends BaseCatalogPropertiesMetada public static final String PAIMON_METASTORE = "metastore"; public static final String WAREHOUSE = "warehouse"; public static final String URI = "uri"; + public static final String JDBC_USER = "jdbc.user"; Review Comment: Paimon use `jdbc.` as property prefix. https://github.com/apache/paimon/blob/master/paimon-core/src/main/java/org/apache/paimon/jdbc/JdbcCatalog.java#L70 -- 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] [#4953] feat(hudi-catalog): add Hudi catalog IT and enable module [gravitino]
mchades commented on code in PR #4965:
URL: https://github.com/apache/gravitino/pull/4965#discussion_r1797632676
##
server/src/main/java/org/apache/gravitino/server/GravitinoServer.java:
##
@@ -119,6 +120,7 @@ protected void configure() {
register(JsonParseExceptionMapper.class);
register(JsonMappingExceptionMapper.class);
register(ObjectMapperProvider.class).register(JacksonFeature.class);
+property(CommonProperties.JSON_JACKSON_DISABLED_MODULES,
"DefaultScalaModule");
Review Comment:
To resolve below error in embedded test mode:
> com.fasterxml.jackson.databind.JsonMappingException: Scala module 2.14.2
requires Jackson Databind version >= 2.14.0 and < 2.15.0 - Found
jackson-databind version 2.15.2
>at
org.apache.hudi.com.fasterxml.jackson.module.scala.JacksonModule.setupModule(JacksonModule.scala:61)
>at
org.apache.hudi.com.fasterxml.jackson.module.scala.JacksonModule.setupModule$(JacksonModule.scala:46)
>at
org.apache.hudi.com.fasterxml.jackson.module.scala.DefaultScalaModule.setupModule(DefaultScalaModule.scala:17)
>at
com.fasterxml.jackson.databind.ObjectMapper.registerModule(ObjectMapper.java:879)
>at
com.fasterxml.jackson.databind.ObjectMapper.registerModules(ObjectMapper.java:1081)
>at
org.glassfish.jersey.jackson.internal.DefaultJacksonJaxbJsonProvider.findAndRegisterModules(DefaultJacksonJaxbJsonProvider.java:86)
>at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>at java.lang.reflect.Method.invoke(Method.java:498)
>at
org.glassfish.hk2.utilities.reflection.ReflectionHelper.invoke(ReflectionHelper.java:1268)
>at
org.jvnet.hk2.internal.ClazzCreator.postConstructMe(ClazzCreator.java:309)
>at org.jvnet.hk2.internal.ClazzCreator.create(ClazzCreator.java:351)
>at
org.jvnet.hk2.internal.SystemDescriptor.create(SystemDescriptor.java:463)
>at
org.jvnet.hk2.internal.SingletonContext$1.compute(SingletonContext.java:59)
>at
org.jvnet.hk2.internal.SingletonContext$1.compute(SingletonContext.java:47)
>at
org.glassfish.hk2.utilities.cache.Cache$OriginThreadAwareFuture$1.call(Cache.java:74)
>at java.util.concurrent.FutureTask.run(FutureTask.java:266)
>at
org.glassfish.hk2.utilities.cache.Cache$OriginThreadAwareFuture.run(Cache.java:131)
>at org.glassfish.hk2.utilities.cache.Cache.compute(Cache.java:176)
>at
org.jvnet.hk2.internal.SingletonContext.findOrCreate(SingletonContext.java:98)
>at org.jvnet.hk2.internal.Utilities.createService(Utilities.java:2102)
>at
org.jvnet.hk2.internal.ServiceHandleImpl.getService(ServiceHandleImpl.java:93)
>at
org.jvnet.hk2.internal.ServiceHandleImpl.getService(ServiceHandleImpl.java:67)
>at
org.glassfish.jersey.inject.hk2.AbstractHk2InjectionManager.lambda$getAllServiceHolders$0(AbstractHk2InjectionManager.java:137)
>at
java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
>at
java.util.LinkedList$LLSpliterator.forEachRemaining(LinkedList.java:1235)
>at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:482)
>at
java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:472)
>at
java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
>at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
>at
java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:566)
>at
org.glassfish.jersey.inject.hk2.AbstractHk2InjectionManager.getAllServiceHolders(AbstractHk2InjectionManager.java:141)
>at
org.glassfish.jersey.inject.hk2.ImmediateHk2InjectionManager.getAllServiceHolders(ImmediateHk2InjectionManager.java:30)
>at
org.glassfish.jersey.internal.inject.Providers.getServiceHolders(Providers.java:307)
>at
org.glassfish.jersey.internal.inject.Providers.getCustomProviders(Providers.java:151)
>at
org.glassfish.jersey.message.internal.MessageBodyFactory.initialize(MessageBodyFactory.java:219)
>at
org.glassfish.jersey.message.internal.MessageBodyFactory$MessageBodyWorkersConfigurator.postInit(MessageBodyFactory.java:114)
>at
org.glassfish.jersey.server.ApplicationHandler.lambda$initialize$2(ApplicationHandler.java:353)
>at java.util.Arrays$ArrayList.forEach(Arrays.java:3880)
>at
org.glassfish.jersey.server.ApplicationHandler.initialize(ApplicationHandler.java:353)
>at
org.glassfish.jersey.server.ApplicationHandler.lambda$initialize$1(ApplicationHandler.java:297)
>at org.glassfish.jersey.internal.Errors.process(Errors.java:292)
>at org.glassfish.jersey.internal.Errors.process(Errors.java:274)
>at
org.glassfish.jersey.internal.Errors.processW
[PR] [#5120] improvement(docs): Add the document of metadata object ownership [gravitino]
jerqi opened a new pull request, #5123: URL: https://github.com/apache/gravitino/pull/5123 ### What changes were proposed in this pull request? Add the document of metadata object ownership ### Why are the changes needed? Fix: #5120 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Just document. -- 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] [#3920] feat(catalog-lakehouse-paimon): Support jdbc backend for Paimon Catalog [gravitino]
caican00 commented on code in PR #5087:
URL: https://github.com/apache/gravitino/pull/5087#discussion_r1797642532
##
catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/utils/CatalogUtils.java:
##
@@ -121,6 +123,17 @@ private static void checkPaimonConfig(PaimonConfig
paimonConfig) {
Preconditions.checkArgument(
StringUtils.isNotBlank(uri), "Paimon Catalog uri can not be null or
empty.");
}
+
+if (PaimonCatalogBackend.JDBC.name().equalsIgnoreCase(metastore)) {
+ String jdbcUser = paimonConfig.get(CATALOG_JDBC_USER);
Review Comment:
fixed.
##
catalogs/catalog-lakehouse-paimon/src/test/java/org/apache/gravitino/catalog/lakehouse/paimon/integration/test/CatalogPaimonFileSystemIT.java:
##
@@ -48,4 +55,22 @@ protected Map initPaimonCatalogProperties() {
return catalogProperties;
}
+
+ @Test
+ void testPaimonSchemaProperties() throws Catalog.DatabaseNotExistException {
Review Comment:
removed.
--
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] [#3920] feat(catalog-lakehouse-paimon): Support jdbc backend for Paimon Catalog [gravitino]
caican00 commented on PR #5087: URL: https://github.com/apache/gravitino/pull/5087#issuecomment-2408476354 > @caican00 , some minor comments, and could you add related document? @FANNG1 please review 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: [PR] [#3920] feat(catalog-lakehouse-paimon): Support jdbc backend for Paimon Catalog [gravitino]
FANNG1 commented on code in PR #5087: URL: https://github.com/apache/gravitino/pull/5087#discussion_r1797643075 ## catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonCatalogPropertiesMetadata.java: ## @@ -46,14 +46,26 @@ public class PaimonCatalogPropertiesMetadata extends BaseCatalogPropertiesMetada public static final String PAIMON_METASTORE = "metastore"; public static final String WAREHOUSE = "warehouse"; public static final String URI = "uri"; + public static final String JDBC_USER = "jdbc.user"; Review Comment: Iceberg internally uses `jdbc.user` too, we'd better provide a unified Gravitino Jdbc username. -- 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] [#3920] feat(catalog-lakehouse-paimon): Support jdbc backend for Paimon Catalog [gravitino]
FANNG1 commented on code in PR #5087:
URL: https://github.com/apache/gravitino/pull/5087#discussion_r1797643309
##
catalogs/catalog-lakehouse-paimon/build.gradle.kts:
##
@@ -122,12 +122,14 @@ dependencies {
testImplementation(libs.junit.jupiter.api)
testImplementation(libs.mysql.driver)
testImplementation(libs.postgresql.driver)
+ testImplementation(libs.h2db)
testImplementation(libs.bundles.log4j)
testImplementation(libs.junit.jupiter.params)
testImplementation(libs.paimon.s3)
testImplementation(libs.paimon.spark)
testImplementation(libs.testcontainers)
testImplementation(libs.testcontainers.localstack)
+ testImplementation(libs.testcontainers.mysql)
Review Comment:
ok
--
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] [#4953] feat(hudi-catalog): add Hudi catalog IT and enable module [gravitino]
mchades commented on code in PR #4965:
URL: https://github.com/apache/gravitino/pull/4965#discussion_r1797630170
##
catalogs/catalog-lakehouse-hudi/build.gradle.kts:
##
@@ -138,3 +150,60 @@ dependencies {
testRuntimeOnly("org.apache.hudi:hudi-spark$sparkVersion-bundle_$scalaVersion:0.15.0")
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
Re: [PR] [#3973] improvement(core):support audit log [gravitino]
FANNG1 commented on PR #4575: URL: https://github.com/apache/gravitino/pull/4575#issuecomment-2408453321 > I kind of agree with @hanwxx , AuditLog and event are two things, we don't have to use interface to put them together. Using a converter to bridge them together without hard combination seems better to me. It will hard to transform `IcebergRESTServiceEvent` to `AuditLog` when supporting Iceberg REST server audit log. `IcebergRESTServiceEvent` will be defined in `IcebergRESTServer` module because it may contains some Iceberg specific object like `TableMetaData`. I agree that we'd better not mix event and audit, but maybe we need a trade-off. -- 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] [#3920] feat(catalog-lakehouse-paimon): Support jdbc backend for Paimon Catalog [gravitino]
caican00 commented on code in PR #5087:
URL: https://github.com/apache/gravitino/pull/5087#discussion_r1797640061
##
catalogs/catalog-lakehouse-paimon/build.gradle.kts:
##
@@ -122,12 +122,14 @@ dependencies {
testImplementation(libs.junit.jupiter.api)
testImplementation(libs.mysql.driver)
testImplementation(libs.postgresql.driver)
+ testImplementation(libs.h2db)
testImplementation(libs.bundles.log4j)
testImplementation(libs.junit.jupiter.params)
testImplementation(libs.paimon.s3)
testImplementation(libs.paimon.spark)
testImplementation(libs.testcontainers)
testImplementation(libs.testcontainers.localstack)
+ testImplementation(libs.testcontainers.mysql)
Review Comment:
> why adding this?
to test paimon catalog when the catalog backend is mysql.
--
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
