You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@seatunnel.apache.org by GitBox <gi...@apache.org> on 2022/02/16 10:49:47 UTC
[GitHub] [incubator-seatunnel] simon824 opened a new pull request #1256: [Feature] [connector] Support spark hdfs source
simon824 opened a new pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256
## Purpose of this pull request
closed #1255
## Check list
* [ ] Code changed are covered with tests, or it does not need tests for reason:
* [ ] If any new Jar binary package adding in you PR, please add License Notice according
[New License Guide](https://github.com/apache/incubator-seatunnel/blob/dev/docs/en/developement/NewLicenseGuide.md)
* [ ] If necessary, please update the documentation to describe the new feature. https://github.com/apache/incubator-seatunnel/tree/dev/docs
--
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
[GitHub] [incubator-seatunnel] simon824 commented on pull request #1256: [Feature] [connector] Support spark hdfs source
Posted by GitBox <gi...@apache.org>.
simon824 commented on pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256#issuecomment-1043750940
> hi, thanks for your contribution, but we already have a `files` source, can you describe the difference between the two
`File source connector` now only supports the prefix `file://`, this pr refers to `File sink` to extend support for hdfs.
**Before**:
**After**:
--
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
[GitHub] [incubator-seatunnel] wuchunfu commented on pull request #1256: [Feature] [connector] Remove `Hdfs Connector` and make it compatible with `File Connector`
Posted by GitBox <gi...@apache.org>.
wuchunfu commented on pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256#issuecomment-1049898260
@simon824 It looks good on the whole, but the `sink plugin` doesn't seem to need the `common-options` option, please check it. Another point is to test the `source plugin` and `sink plugin` , thank you
--
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
[GitHub] [incubator-seatunnel] CalvinKirs commented on pull request #1256: [Feature] [connector] Support spark hdfs source
Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256#issuecomment-1043735476
hi, thanks for your contribution, but we already have a `files` source, can you describe the difference between the two
--
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
[GitHub] [incubator-seatunnel] wuchunfu commented on a change in pull request #1256: [Feature] [connector] Support spark hdfs source
Posted by GitBox <gi...@apache.org>.
wuchunfu commented on a change in pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256#discussion_r808115696
##########
File path: seatunnel-connectors/seatunnel-connector-spark-file/src/main/scala/org/apache/seatunnel/spark/source/Hdfs.scala
##########
@@ -0,0 +1,32 @@
+/*
+ * 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.seatunnel.spark.source
+
+import org.apache.seatunnel.common.config.CheckResult
+import org.apache.seatunnel.spark.SparkEnvironment
+import org.apache.spark.sql.{Dataset, Row}
+
+class Hdfs extends FileSourceBase {
+
+ override def checkConfig(): CheckResult = {
+ checkConfigImpl("hdfs://")
+ }
Review comment:
Wouldn't it be better to extract `hdfs://` as a constant?
--
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
[GitHub] [incubator-seatunnel] wuchunfu commented on pull request #1256: [Feature] [connector] Remove `Hdfs Connector` and make it compatible with `File Connector`
Posted by GitBox <gi...@apache.org>.
wuchunfu commented on pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256#issuecomment-1050486984
> hi @wuchunfu ,i found the option`source_table_name` is in `common-options` , do we need it? FYI: https://github.com/apache/incubator-seatunnel/blob/dev/docs/en/spark/configuration/sink-plugins/sink-plugin.md
@simon824 Oh, maybe I'm mistaken, this seems to be optional.
--
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
[GitHub] [incubator-seatunnel] simon824 commented on pull request #1256: [Feature] [connector] Remove `Hdfs Connector` and make `File Connector` and hdfs compatible
Posted by GitBox <gi...@apache.org>.
simon824 commented on pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256#issuecomment-1049401122
> > @wuchunfu make sense, i will check it later and achieve it next pr.
>
> It would be even better if it could be implemented in this PR.
All right.
--
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
[GitHub] [incubator-seatunnel] CalvinKirs commented on pull request #1256: [Feature] [connector] Support spark hdfs source
Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256#issuecomment-1048803514
> @wuchunfu make sense, i will check it later and achieve it next pr.
It would be even better if it could be implemented in this PR.
--
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
[GitHub] [incubator-seatunnel] simon824 commented on pull request #1256: [Feature] [connector] Support spark hdfs source
Posted by GitBox <gi...@apache.org>.
simon824 commented on pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256#issuecomment-1042491497
Updated, thanks a lot for your review @wuchunfu , please check it again when free 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-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] wuchunfu commented on pull request #1256: [Feature] [connector] Remove `Hdfs Connector` and make it compatible with `File Connector`
Posted by GitBox <gi...@apache.org>.
wuchunfu commented on pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256#issuecomment-1050487216
LGTM
--
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
[GitHub] [incubator-seatunnel] wuchunfu commented on a change in pull request #1256: [Feature] [connector] Support spark hdfs source
Posted by GitBox <gi...@apache.org>.
wuchunfu commented on a change in pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256#discussion_r808113143
##########
File path: seatunnel-connectors/seatunnel-connector-spark-file/src/main/scala/org/apache/seatunnel/spark/source/FileSourceBase.scala
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.seatunnel.spark.source
+
+import scala.collection.JavaConversions._
+import scala.util.{Failure, Success, Try}
+
+import org.apache.seatunnel.common.config.{CheckResult, TypesafeConfigUtils}
+import org.apache.seatunnel.common.config.CheckConfigUtil.checkAllExists
+import org.apache.seatunnel.spark.SparkEnvironment
+import org.apache.seatunnel.spark.batch.{SparkBatchSink, SparkBatchSource}
+import org.apache.spark.sql.{Dataset, Row, SparkSession}
+
+abstract class FileSourceBase extends SparkBatchSource {
+
+ override def prepare(env: SparkEnvironment): Unit = {}
+
+ def getDataImpl(env: SparkEnvironment, schema: String): Dataset[Row] = {
+ val path = buildPathWithDefaultSchema(config.getString("path"), schema)
+ fileReader(env.getSparkSession, path)
+ }
+
+ def checkConfigImpl(schema: String): CheckResult = {
+ val checkResult = checkAllExists(config, "path")
+ if (checkResult.isSuccess) {
+ val dir = config.getString("path")
+ dir.startsWith("/") || dir.startsWith(schema) match {
+ case false =>
+ CheckResult.error("invalid path URI, please set the following allowed schemas: " + schema)
+ case _ => checkResult
+ }
+ } else {
+ checkResult
+ }
+ }
+
+ protected def fileReader(spark: SparkSession, path: String): Dataset[Row] = {
+ val format = config.getString("format")
+ var reader = spark.read.format(format)
Review comment:
The `format` parameter doesn't seem to be described in the documentation
--
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
[GitHub] [incubator-seatunnel] wuchunfu commented on a change in pull request #1256: [Feature] [connector] Support spark hdfs source
Posted by GitBox <gi...@apache.org>.
wuchunfu commented on a change in pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256#discussion_r808110897
##########
File path: docs/en/spark/configuration/source-plugins/Hdfs.md
##########
@@ -0,0 +1,50 @@
+# Hdfs
+
+> Source plugin : Hdfs [Spark]
+
+## Description
+Export data to HDFS
+
+## Options
+
+| name | type | required | default value |
+| --- | --- | --- | --- |
+| options| object | no | - |
+| partition_by | array | no | - |
+| path | string | yes | - |
+| save_mode | string | no | error |
+| serializer | string | no | json |
+| common-options | string | no | - |
+
+#### options [object]
+
+Custom parameters
+
+#### partition_by [array]
+
+Partition data based on selected fields
Review comment:
source plugin doesn't seem to need this parameter
--
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
[GitHub] [incubator-seatunnel] wuchunfu commented on pull request #1256: [Feature] [connector] Support spark hdfs source
Posted by GitBox <gi...@apache.org>.
wuchunfu commented on pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256#issuecomment-1047840951
> > > hi, thanks for your contribution, but we already have a `files` source, can you describe the difference between the two
> >
> >
> > `File source connector` now only supports the prefix `file://`, this pr refers to `File sink` to extend support for hdfs.
> > **Before**: 
> > **After**: 
>
> If it's just this change, we might be better off making changes to the `files` plugin instead of adding a plugin.
@simon824 Can the `sink plugin` also be like the `source plugin` , without distinguishing in the code whether it is `file://` or `hdfs://`.
--
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
[GitHub] [incubator-seatunnel] simon824 commented on pull request #1256: [Feature] [connector] Remove `Hdfs Connector` and make it compatible with `File Connector`
Posted by GitBox <gi...@apache.org>.
simon824 commented on pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256#issuecomment-1049524814
Updated, PTAL again when free thanks. @wuchunfu @CalvinKirs
--
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
[GitHub] [incubator-seatunnel] wuchunfu commented on a change in pull request #1256: [Feature] [connector] Support spark hdfs source
Posted by GitBox <gi...@apache.org>.
wuchunfu commented on a change in pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256#discussion_r808111197
##########
File path: docs/en/spark/configuration/source-plugins/Hdfs.md
##########
@@ -0,0 +1,50 @@
+# Hdfs
+
+> Source plugin : Hdfs [Spark]
+
+## Description
+Export data to HDFS
+
+## Options
+
+| name | type | required | default value |
+| --- | --- | --- | --- |
+| options| object | no | - |
+| partition_by | array | no | - |
+| path | string | yes | - |
+| save_mode | string | no | error |
+| serializer | string | no | json |
+| common-options | string | no | - |
+
+#### options [object]
+
+Custom parameters
+
+#### partition_by [array]
+
+Partition data based on selected fields
+
+#### path [string]
+
+Output file path, starting with **hdfs://**
+
+#### save_mode [string]
+
+Storage mode, currently supports overwrite, append, ignore and error. For the specific meaning of each mode, see [save-modes](http://spark.apache.org/docs/2.2.0/sql-programming-guide.html#save-modes)
Review comment:
source plugin doesn't seem to need this parameter
--
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
[GitHub] [incubator-seatunnel] simon824 commented on pull request #1256: [Feature] [connector] Support spark hdfs source
Posted by GitBox <gi...@apache.org>.
simon824 commented on pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256#issuecomment-1045832280
>if it's just this change, we might be better off making changes to the files plugin instead of adding a plugin.
@CalvinKirs make sense , i updated supporting for `hdfs` base on `File sink`, PTAL when free 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-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] CalvinKirs commented on pull request #1256: [Feature] [connector] Support spark hdfs source
Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256#issuecomment-1044225948
> > hi, thanks for your contribution, but we already have a `files` source, can you describe the difference between the two
>
> `File source connector` now only supports the prefix `file://`, this pr refers to `File sink` to extend support for hdfs.
>
> **Before**: 
>
> **After**: 
If it's just this change, we might be better off making changes to the `files` plugin instead of adding a plugin.
--
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
[GitHub] [incubator-seatunnel] wuchunfu commented on a change in pull request #1256: [Feature] [connector] Support spark hdfs source
Posted by GitBox <gi...@apache.org>.
wuchunfu commented on a change in pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256#discussion_r808124763
##########
File path: docs/en/spark/configuration/source-plugins/Hdfs.md
##########
@@ -0,0 +1,50 @@
+# Hdfs
+
+> Source plugin : Hdfs [Spark]
+
+## Description
+Export data to HDFS
+
+## Options
+
+| name | type | required | default value |
+| --- | --- | --- | --- |
+| options| object | no | - |
+| partition_by | array | no | - |
+| path | string | yes | - |
+| save_mode | string | no | error |
+| serializer | string | no | json |
+| common-options | string | no | - |
+
+#### options [object]
+
+Custom parameters
+
+#### partition_by [array]
+
+Partition data based on selected fields
+
+#### path [string]
+
+Output file path, starting with **hdfs://**
+
+#### save_mode [string]
+
+Storage mode, currently supports overwrite, append, ignore and error. For the specific meaning of each mode, see [save-modes](http://spark.apache.org/docs/2.2.0/sql-programming-guide.html#save-modes)
+
+#### serializer [string]
+
+Serialization method, currently supports csv, json, parquet, orc and text
+
+### common options [string]
+
+Source Plugin common parameters, refer to [Source Plugin](./source-plugin.md) for details
Review comment:
The `result_table_name` parameter seems to be `required` , please check
--
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
[GitHub] [incubator-seatunnel] CalvinKirs commented on pull request #1256: [Feature] [connector] Support spark hdfs source
Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256#issuecomment-1047649481
@wuchunfu Hi, PTAL again, thx
--
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
[GitHub] [incubator-seatunnel] CalvinKirs commented on pull request #1256: [Feature] [connector] Support spark hdfs source
Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256#issuecomment-1041414323
@wuchunfu hi,PTAL,thx
--
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
[GitHub] [incubator-seatunnel] simon824 commented on pull request #1256: [Feature] [connector] Support spark hdfs source
Posted by GitBox <gi...@apache.org>.
simon824 commented on pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256#issuecomment-1048369530
@wuchunfu make sense, i will check it later and achieve it next pr.
--
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
[GitHub] [incubator-seatunnel] simon824 commented on pull request #1256: [Feature] [connector] Remove `Hdfs Connector` and make it compatible with `File Connector`
Posted by GitBox <gi...@apache.org>.
simon824 commented on pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256#issuecomment-1050426315
hi @wuchunfu ,i found the option`source_table_name` is in `common-options` , do we need it?
FYI: https://github.com/apache/incubator-seatunnel/blob/dev/docs/en/spark/configuration/sink-plugins/sink-plugin.md
--
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
[GitHub] [incubator-seatunnel] CalvinKirs merged pull request #1256: [Feature] [connector] Remove `Hdfs Connector` and make it compatible with `File Connector`
Posted by GitBox <gi...@apache.org>.
CalvinKirs merged pull request #1256:
URL: https://github.com/apache/incubator-seatunnel/pull/1256
--
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