Copilot commented on code in PR #12641:
URL: https://github.com/apache/gravitino/pull/12641#discussion_r4060852350
##########
flink-connector/flink-common/src/main/java/org/apache/gravitino/flink/connector/iceberg/GravitinoIcebergCatalogFactory.java:
##########
@@ -58,13 +58,10 @@ protected Catalog newCatalog(
PartitionConverter partitionConverter,
Map<String, String> catalogOptions,
Map<String, String> icebergCatalogProperties) {
- return new GravitinoIcebergCatalog(
- catalogName,
- defaultDatabase,
- schemaAndTablePropertiesConverter,
- partitionConverter,
- catalogOptions,
- icebergCatalogProperties);
+ // GravitinoIcebergCatalog is abstract (its inner-catalog construction
differs per Flink
+ // version); every concrete, version-specific factory overrides this hook.
+ throw new UnsupportedOperationException(
+ "newCatalog() must be overridden by a Flink version-specific catalog
factory");
Review Comment:
Since `GravitinoIcebergCatalogFactory` remains a concrete public class,
`createCatalog()` now exposes a factory that always throws
`UnsupportedOperationException`; this is a behavior-breaking change for direct
factory users. Make the base factory abstract and adapt the helper test, or
retain a default implementation rather than leaving a concrete `CatalogFactory`
that cannot create catalogs.
##########
flink-connector/v2.1/flink/build.gradle.kts:
##########
@@ -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.
+ */
+plugins {
+ `maven-publish`
+ id("java")
+ id("idea")
+}
+
+repositories {
+ mavenCentral()
+}
+
+// flink-common's sources are compiled directly into this module's own
sourceSet (in addition
+// to flink-common's own jar) so that they are re-checked against this
module's own
+// Flink/Iceberg/Paimon dependency versions (this matters once versions
diverge in incompatible
+// ways, e.g. Flink 2.x removing APIs that are still present in the 1.x line).
+val commonProject = project(":flink-connector:flink-common")
+
+sourceSets {
+ main {
+ java.srcDir(commonProject.file("src/main/java"))
+ }
+ test {
+ java.srcDir(commonProject.file("src/test/java"))
+ resources.srcDir(commonProject.file("src/test/resources"))
+ }
+}
+
+// Spotless can't format files outside the project dir; flink-common's own
sources are linted
+// by the flink-common project itself, so restrict this project's spotless
target to its own.
+plugins.withType<com.diffplug.gradle.spotless.SpotlessPlugin>().configureEach {
+ configure<com.diffplug.gradle.spotless.SpotlessExtension> {
+ java {
+ target("src/**/*.java")
+ }
+ }
+}
+
+val flinkVersion: String = libs.versions.flink21.get()
+val flinkMajorVersion: String = flinkVersion.substringBeforeLast(".")
+val icebergVersion: String = libs.versions.iceberg4flink21.get()
+val paimonVersion: String = libs.versions.paimon4flink21.get()
+// Flink 2.x removed the Scala APIs entirely, so unlike the 1.x modules there
is no scala suffix.
+val artifactName = "${rootProject.name}-flink-$flinkMajorVersion"
+
+dependencies {
+ constraints {
+ // Force upgrade for outdated transitive libthrift pulled by Hive Metastore
+ compileOnly(libs.thrift)
+ testImplementation(libs.thrift)
+ // flink-connector-jdbc-core transitively pulls
openlineage-sql-java:1.32.0, whose bundled,
+ // unrelocated org.apache.commons.lang3.SystemProperties predates the
getUserName(String)
+ // overload and shadows the real commons-lang3 on the classpath
(NoSuchMethodError). This
+ // can't simply be excluded: the connector's lineage extraction actually
calls into it at
+ // runtime (JdbcSource.getLineageVertex). Force this newer release
instead, which ships an
+ // up-to-date bundled copy.
+ compileOnly(libs.openlineageSqlJava21)
+ testImplementation(libs.openlineageSqlJava21)
+ }
Review Comment:
The workaround described here is not present in the published runtime
artifact: this only constrains the `compileOnly` and test configurations, while
`flink-runtime-2.1` builds its shadow JAR from the runtime classpath
(`flink-connector/v2.1/flink-runtime/build.gradle.kts:46-59`). Consequently the
JDBC connector's transitive `openlineage-sql-java:1.32.0` can still be the
class loaded in a Flink deployment, so the documented
`SystemProperties.getUserName(String)` `NoSuchMethodError` remains possible.
Add `openlineageSqlJava21` as a runtime/shadow dependency (or otherwise ensure
1.52.0 is shipped).
--
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]
