[GitHub] [incubator-seatunnel] EricJoy2048 commented on a diff in pull request #2655: [Improve][Connector-V2] Refactor local file sink connector code structure

[GitBox <gi...@apache.org>]

EricJoy2048 commented on code in PR #2655:
URL: https://github.com/apache/incubator-seatunnel/pull/2655#discussion_r963297469


##########
seatunnel-connectors-v2/connector-file/connector-file-local/src/main/java/org/apache/seatunnel/connectors/seatunnel/file/local/sink/LocalFileSink.java:
##########
@@ -17,16 +17,26 @@
 
 package org.apache.seatunnel.connectors.seatunnel.file.local.sink;
 
+import org.apache.seatunnel.api.common.PrepareFailException;
 import org.apache.seatunnel.api.sink.SeaTunnelSink;
-import org.apache.seatunnel.connectors.seatunnel.file.sink.AbstractFileSink;
-import org.apache.seatunnel.connectors.seatunnel.file.sink.spi.SinkFileSystemPlugin;
+import org.apache.seatunnel.connectors.seatunnel.file.config.FileSystemType;
+import org.apache.seatunnel.connectors.seatunnel.file.sink.BaseFileSink;
+
+import org.apache.seatunnel.shade.com.typesafe.config.Config;
 
 import com.google.auto.service.AutoService;
 
 @AutoService(SeaTunnelSink.class)
-public class LocalFileSink extends AbstractFileSink {
+public class LocalFileSink extends BaseFileSink {
+
+    @Override
+    public String getPluginName() {
+        return FileSystemType.LOCAL.getFileSystemPluginName();
+    }
+
     @Override
-    public SinkFileSystemPlugin getSinkFileSystemPlugin() {
-        return new LocalFileSinkPlugin();
+    public void prepare(Config pluginConfig) throws PrepareFailException {
+        super.prepare(pluginConfig);
+        hadoopConf = null;

Review Comment:
   > Yes, this is because the local file system needs to generate an empty `Configuration` object and does not need the help of a `HadoopConf` object.
   > 
   > But there is another approach, instantiate HadoopConf, the property fsHdfsImpl set to `org.apache.hadoop.fs.LocalFileSystem`.
   > 
   > Which of these two options do you think is better?
   
   Because currently all file systems are accessed through HDFS protocol. So I think `But there is another approach, instantiate HadoopConf, the property fsHdfsImpl set to org.apache.hadoop.fs.LocalFileSystem` is better. This makes the overall design look clearer.



-- 
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-unsubscribe@seatunnel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org