Copilot commented on code in PR #9147: URL: https://github.com/apache/gravitino/pull/9147#discussion_r2568380970
########## bundles/iceberg-aliyun-bundle/build.gradle.kts: ########## @@ -0,0 +1,51 @@ +/* + * 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 com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar + +plugins { + `maven-publish` + id("java") + alias(libs.plugins.shadow) +} + +dependencies { + implementation(libs.aliyun.credentials.sdk) + implementation(libs.aliyun.sdk.oss) + // Aliyun oss SDK depends on this package, and JDK >= 9 requires manual add + // https://www.alibabacloud.com/help/en/oss/developer-reference/java-installation?spm=a2c63.p38356.0.i1 + implementation(libs.sun.activation) +} + +tasks.withType(ShadowJar::class.java) { + isZip64 = true + configurations = listOf(project.configurations.runtimeClasspath.get()) + archiveClassifier.set("") + mergeServiceFiles() + + dependencies { + exclude(dependency("org.slf4j:slf4j-api")) + } + + mergeServiceFiles() Review Comment: The `mergeServiceFiles()` call is duplicated on lines 39 and 45. Remove one of the duplicate calls. ```suggestion ``` ########## bundles/iceberg-aws-bundle/build.gradle.kts: ########## @@ -0,0 +1,51 @@ +/* + * 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 Review Comment: Inconsistent formatting in the license header. Lines 14-15 have "OF ANY" split across two lines, while other files in this PR keep "OF ANY KIND," on a single line. This should be corrected for consistency. ```suggestion * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the ``` ########## docs/security/credential-vending.md: ########## @@ -168,16 +168,12 @@ For Fileset catalog, please use Gravitino cloud bundle jar with Hadoop and cloud - [Gravitino GCP bundle jar with Hadoop and cloud packages](https://mvnrepository.com/artifact/org.apache.gravitino/gravitino-gcp-bundle) - [Gravitino Azure bundle jar with Hadoop and cloud packages](https://mvnrepository.com/artifact/org.apache.gravitino/gravitino-azure-bundle) -For Iceberg REST catalog server, please use the Gravitino cloud bundle jar without Hadoop and cloud packages. Additionally, download the corresponding Iceberg cloud packages. +For Iceberg REST catalog server, please download the corresponding Gravitino cloud packages. -- [Gravitino AWS bundle jar without Hadoop and cloud packages](https://mvnrepository.com/artifact/org.apache.gravitino/gravitino-aws) -- [Gravitino Aliyun bundle jar without Hadoop and cloud packages](https://mvnrepository.com/artifact/org.apache.gravitino/gravitino-aliyun) -- [Gravitino GCP bundle jar without Hadoop and cloud packages](https://mvnrepository.com/artifact/org.apache.gravitino/gravitino-gcp) -- [Gravitino Azure bundle jar without Hadoop and cloud packages](https://mvnrepository.com/artifact/org.apache.gravitino/gravitino-azure) - -:::note -For OSS, Iceberg doesn't provide Iceberg Aliyun bundle jar which contains OSS packages, you could provide the OSS jar by yourself or use [Gravitino Aliyun bundle jar with Hadoop and cloud packages](https://mvnrepository.com/artifact/org.apache.gravitino/gravitino-aliyun-bundle), please refer to [OSS configuration](../iceberg-rest-service.md#oss-configuration) for more details. -::: +- [Gravitino Iceberg AWS bundle JAR](https://mvnrepository.com/artifact/org.apache.gravitino/gravitino-aws-bundle) +- [Gravitino Iceberg GCP bundle JAR](https://mvnrepository.com/artifact/com.github.apache.incubator-gravitino/gravitino-gcp-bundle) +- [Gravitino Iceberg Aliyun bundle JAR](https://mvnrepository.com/artifact/com.github.apache.gravitino/gravitino-aliyun-bundle) +- [Gravitino Iceberg Azure bundle JAR](https://mvnrepository.com/artifact/com.github.apache.gravitino/gravitino-azure-bundle) Review Comment: Inconsistent artifact names in the Maven repository links. The artifact IDs appear to be incorrect: - Line 174: `com.github.apache.incubator-gravitino/gravitino-gcp-bundle` should be `org.apache.gravitino/gravitino-iceberg-gcp-bundle` - Line 175: `com.github.apache.incubator-gravitino/gravitino-aliyun-bundle` should be `org.apache.gravitino/gravitino-iceberg-aliyun-bundle` - Line 176: `com.github.apache.incubator-gravitino/gravitino-azure-bundle` should be `org.apache.gravitino/gravitino-iceberg-azure-bundle` These should match the actual Maven artifacts that will be published and be consistent with the naming convention used elsewhere (e.g., line 173 uses `org.apache.gravitino/gravitino-aws-bundle`). ```suggestion - [Gravitino Iceberg GCP bundle JAR](https://mvnrepository.com/artifact/org.apache.gravitino/gravitino-iceberg-gcp-bundle) - [Gravitino Iceberg Aliyun bundle JAR](https://mvnrepository.com/artifact/org.apache.gravitino/gravitino-iceberg-aliyun-bundle) - [Gravitino Iceberg Azure bundle JAR](https://mvnrepository.com/artifact/org.apache.gravitino/gravitino-iceberg-azure-bundle) ``` ########## dev/docker/iceberg-rest-server/iceberg-rest-server-dependency.sh: ########## @@ -50,35 +50,18 @@ tar xfz gravitino-iceberg-rest-server-*.tar.gz cp -r gravitino-iceberg-rest-server*-bin ${iceberg_rest_server_dir}/packages/gravitino-iceberg-rest-server cd ${gravitino_home} -./gradlew :bundles:gcp:jar -./gradlew :bundles:aws:jar -./gradlew :bundles:azure:jar -./gradlew :bundles:aliyun:jar +./gradlew :bundles:iceberg-gcp-bundle:shadowJar +./gradlew :bundles:iceberg-aws-bundle:shadowJar +./gradlew :bundles:iceberg-azure-bundle:shadowJar +./gradlew :bundles:iceberg-aliyun-bundle:shadowJar # prepare bundle jar cd ${iceberg_rest_server_dir} mkdir -p bundles -find ${gravitino_home}/bundles/gcp/build/libs/ -name 'gravitino-gcp-*.jar' ! -name '*-empty.jar' -exec cp -v {} bundles/ \; -find ${gravitino_home}/bundles/aws/build/libs/ -name 'gravitino-aws-*.jar' ! -name '*-empty.jar' -exec cp -v {} bundles/ \; -find ${gravitino_home}/bundles/azure/build/libs/ -name 'gravitino-azure-*.jar' ! -name '*-empty.jar' -exec cp -v {} bundles/ \; -find ${gravitino_home}/bundles/aliyun/build/libs/ -name 'gravitino-aliyun-*.jar' ! -name '*-empty.jar' -exec cp -v {} bundles/ \; - -iceberg_version="1.10.0" - -iceberg_gcp_bundle="iceberg-gcp-bundle-${iceberg_version}.jar" -if [ ! -f "bundles/${iceberg_gcp_bundle}" ]; then - curl -L -s -o bundles/${iceberg_gcp_bundle} https://repo1.maven.org/maven2/org/apache/iceberg/iceberg-gcp-bundle/${iceberg_version}/${iceberg_gcp_bundle} -fi - -iceberg_aws_bundle="iceberg-aws-bundle-${iceberg_version}.jar" -if [ ! -f "bundles/${iceberg_aws_bundle}" ]; then - curl -L -s -o bundles/${iceberg_aws_bundle} https://repo1.maven.org/maven2/org/apache/iceberg/iceberg-aws-bundle/${iceberg_version}/${iceberg_aws_bundle} -fi - -iceberg_azure_bundle="iceberg-azure-bundle-${iceberg_version}.jar" -if [ ! -f "bundles/${iceberg_azure_bundle}" ]; then - curl -L -s -o bundles/${iceberg_azure_bundle} https://repo1.maven.org/maven2/org/apache/iceberg/iceberg-azure-bundle/${iceberg_version}/${iceberg_azure_bundle} -fi +find ${gravitino_home}/bundles/iceberg-gcp-bundle/build/libs/ -name 'gravitino-iceberg-gcp-bundle-*.jar' ! -name '*-empty.jar' -exec cp -v {} bundles/ \; +find ${gravitino_home}/bundles/iceberg-aws-bundle/build/libs/ -name 'gravitino-iceberg-aws-bundle-*.jar' ! -name '*-empty.jar' -exec cp -v {} bundles/ \; +find ${gravitino_home}/bundles/iceberg-azure-bundle/build/libs/ -name 'gravitino-iceberg-azure-bundle-*.jar' ! -name '*-empty.jar' -exec cp -v {} bundles/ \; +find ${gravitino_home}/bundles/iceberg-aliyun-bundle/build/libs/ -name 'gravitino-iceberg-aliyun-bundle-*.jar' ! -name '*-empty.jar' -exec cp -v {} bundles/ \; Review Comment: Path mismatch in find commands. The gradle tasks build to `:bundles:iceberg-gcp-bundle:shadowJar` (line 53) but the find command looks for files in `bundles/iceberg-gcp/build/libs/` (line 61). The correct path should be `bundles/iceberg-gcp-bundle/build/libs/`. The same issue exists for lines 62-64 (should be `iceberg-aws-bundle`, `iceberg-azure-bundle`, and `iceberg-aliyun-bundle` respectively). ########## docs/security/credential-vending.md: ########## @@ -190,7 +186,7 @@ The classpath of the server: Suppose the Iceberg table data is stored in S3, follow the steps below: -1. Download the [Gravitino AWS bundle jar without hadoop packages](https://mvnrepository.com/artifact/org.apache.gravitino/gravitino-aws), and place it to the classpath of Iceberg REST server. +1. Download the [Iceberg AWS bundle JAR](https://mvnrepository.com/artifact/org.apache.iceberg/iceberg-aws-bundle), and place it in the classpath of Iceberg REST server. Review Comment: Incorrect instructions. The text says "Download the Iceberg AWS bundle JAR" but the link points to the Iceberg AWS bundle, which doesn't match the context. Based on the PR changes, this should be referring to the new Gravitino Iceberg AWS bundle JAR (gravitino-iceberg-aws-bundle), not the upstream Iceberg bundle. The link and description should be updated accordingly. ```suggestion 1. Download the [Gravitino Iceberg AWS bundle JAR](https://mvnrepository.com/artifact/org.apache.gravitino/gravitino-aws-bundle), and place it in the classpath of Iceberg REST server. ``` ########## settings.gradle.kts: ########## @@ -78,10 +78,10 @@ project(":spark-connector:spark-runtime-3.5").projectDir = file("spark-connector include("web:web", "web:integration-test") include("docs") include("integration-test-common") -include(":bundles:aws", ":bundles:aws-bundle") -include(":bundles:gcp", ":bundles:gcp-bundle") -include(":bundles:aliyun", ":bundles:aliyun-bundle") -include(":bundles:azure", ":bundles:azure-bundle") +include(":bundles:aws", ":bundles:aws-bundle", "bundles:iceberg-aws-bundle") Review Comment: Missing colon prefix before "bundles:iceberg-aws-bundle". All other module paths in this include statement use the `:` prefix consistently. ```suggestion include(":bundles:aws", ":bundles:aws-bundle", ":bundles:iceberg-aws-bundle") ``` -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
