XuQianJin-Stars commented on code in PR #3256:
URL: https://github.com/apache/fluss/pull/3256#discussion_r3214230848
##########
fluss-flink/fluss-flink-common/pom.xml:
##########
@@ -95,6 +95,24 @@
<scope>provided</scope>
</dependency>
+ <!-- Hudi -->
Review Comment:
Looking at the existing modules:
- **Paimon** is `import`-ed directly in `LakeFlinkCatalog` /
`LakeTableFactory`, so it requires a pom dep.
- **Iceberg** is loaded via
`Class.forName("org.apache.iceberg.flink.FlinkDynamicTableFactory")` in
`LakeTableFactory#getIcebergFactory()`. **There is no Iceberg dep in
`fluss-flink-common/pom.xml`** — `fluss-flink-common` stays decoupled from
Iceberg, and the actual jars come from `plugins/iceberg/*` at runtime.
- **Hudi** in this PR has no Java references at all (`git grep
"org.apache.hudi" -- '*.java'` returns 0 hits), yet adds a hard pom dep to
`hudi-flink1.20-bundle` — a shaded uber-bundle that re-bundles
hadoop/parquet/avro/jackson/guava. This is the worst of both worlds: dead
dependency + classpath pollution for every downstream that consumes
`fluss-flink-common`.
Suggestion: drop this dep from `fluss-flink-common/pom.xml`. When real
Hudi-Flink integration lands, follow the Iceberg pattern (`Class.forName`) and
keep concrete Hudi artifacts only in `fluss-lake-hudi/pom.xml`.
##########
fluss-lake/fluss-lake-hudi/src/main/java/org/apache/fluss/lake/hudi/HudiLakeStorage.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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.fluss.lake.hudi;
+
+import org.apache.fluss.config.Configuration;
+import org.apache.fluss.lake.lakestorage.LakeCatalog;
+import org.apache.fluss.lake.lakestorage.LakeStorage;
+import org.apache.fluss.lake.source.LakeSource;
+import org.apache.fluss.lake.writer.LakeTieringFactory;
+import org.apache.fluss.metadata.TablePath;
+
+/** Implementation of Hudi lake storage. */
+public class HudiLakeStorage implements LakeStorage {
+
+ protected final Configuration hudiConfig;
Review Comment:
nit (visibility): Please make this private to match
IcebergLakeStorage#icebergConfig and PaimonLakeStorage#paimonConfig. There's no
subclass that needs protected access.
##########
fluss-lake/fluss-lake-hudi/src/main/java/org/apache/fluss/lake/hudi/HudiLakeStorage.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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.fluss.lake.hudi;
+
+import org.apache.fluss.config.Configuration;
+import org.apache.fluss.lake.lakestorage.LakeCatalog;
+import org.apache.fluss.lake.lakestorage.LakeStorage;
+import org.apache.fluss.lake.source.LakeSource;
+import org.apache.fluss.lake.writer.LakeTieringFactory;
+import org.apache.fluss.metadata.TablePath;
+
+/** Implementation of Hudi lake storage. */
Review Comment:
nit (consistency): IcebergLakeStorage uses /** Iceberg implementation of
{@link LakeStorage}. */ and PaimonLakeStorage likewise. For consistency:
/** Hudi implementation of {@link LakeStorage}. */
--
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]
