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/03/03 16:44:42 UTC
[GitHub] [incubator-seatunnel] zhaomin1423 opened a new pull request #1390: [Feature][spark-connector-jdbc] add pre sql and post sql #1386
zhaomin1423 opened a new pull request #1390:
URL: https://github.com/apache/incubator-seatunnel/pull/1390
<!--
Thank you for contributing to SeaTunnel! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
## Contribution Checklist
- Make sure that the pull request corresponds to a [GITHUB issue](https://github.com/apache/incubator-seatunnel/issues).
- Name the pull request in the form "[Feature] [component] Title of the pull request", where *Feature* can be replaced by `Hotfix`, `Bug`, etc.
- Minor fixes should be named following this pattern: `[hotfix] [docs] Fix typo in README.md doc`.
-->
## Purpose of this pull request
add pre sql and post sql #1386
<!-- Describe the purpose of this pull request. For example: This pull request adds checkstyle plugin.-->
## 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] zhaomin1423 commented on pull request #1390: [Feature][spark-connector-jdbc] add pre sql and post sql #1386
Posted by GitBox <gi...@apache.org>.
zhaomin1423 commented on pull request #1390:
URL: https://github.com/apache/incubator-seatunnel/pull/1390#issuecomment-1058866965
> > > But you need to fix the code style of your code, we have scala-style plugin, you can run it locally
> >
> >
> > Where is the checkStyle result? I am confused.
>
> @zhaomin1423 I think you can implement scalastyle in idea
Thanks, I found them.
--
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] asdf2014 commented on pull request #1390: [Feature][spark-connector-jdbc] add pre sql and post sql #1386
Posted by GitBox <gi...@apache.org>.
asdf2014 commented on pull request #1390:
URL: https://github.com/apache/incubator-seatunnel/pull/1390#issuecomment-1060668984
Hi @zhaomin1423, would you please merge the dev branch into this PR to pass the CI, 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] asdf2014 commented on pull request #1390: [Feature][spark-connector-jdbc] add pre sql and post sql #1386
Posted by GitBox <gi...@apache.org>.
asdf2014 commented on pull request #1390:
URL: https://github.com/apache/incubator-seatunnel/pull/1390#issuecomment-1060750158
@zhaomin1423 You are welcome
--
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] zhaomin1423 commented on pull request #1390: [Feature][spark-connector-jdbc] add pre sql and post sql #1386
Posted by GitBox <gi...@apache.org>.
zhaomin1423 commented on pull request #1390:
URL: https://github.com/apache/incubator-seatunnel/pull/1390#issuecomment-1058254321
Hi，Could you like to help me review it? Also, the license checker were not successful. How should I do?
--
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 #1390: [Feature][spark-connector-jdbc] add pre sql and post sql #1386
Posted by GitBox <gi...@apache.org>.
wuchunfu commented on pull request #1390:
URL: https://github.com/apache/incubator-seatunnel/pull/1390#issuecomment-1058845117
> > But you need to fix the code style of your code, we have scala-style plugin, you can run it locally
>
> Where is the checkStyle result? I am confused.
@zhaomin1423 I think you can implement scalastyle in idea
--
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 #1390: [Feature][spark-connector-jdbc] add pre sql and post sql #1386
Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on pull request #1390:
URL: https://github.com/apache/incubator-seatunnel/pull/1390#issuecomment-1058257542
I'll check it out later, there is a problem with the license check, but you don‘t added a new Jar, you can ignore this.
--
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] zhaomin1423 commented on pull request #1390: [Feature][spark-connector-jdbc] add pre sql and post sql #1386
Posted by GitBox <gi...@apache.org>.
zhaomin1423 commented on pull request #1390:
URL: https://github.com/apache/incubator-seatunnel/pull/1390#issuecomment-1058731012
The pre sqls and post sqls are executed in driver, the output is executed in executors. They can not use a connection. The post sqls use a new connection because the output can be running a long time, a new connection is better for using.
| |
***@***.***
|
|
***@***.***
|
---- 回复的原邮件 ----
| 发件人 | ***@***.***> |
| 日期 | 2022年03月04日 08:57 |
| 收件人 | ***@***.***> |
| 抄送至 | Xiao ***@***.******@***.***> |
| 主题 | Re: [apache/incubator-seatunnel] [Feature][spark-connector-jdbc] add pre sql and post sql #1386 (PR #1390) |
@yx91490 commented on this pull request.
In seatunnel-connectors/seatunnel-connector-spark-jdbc/src/main/scala/org/apache/seatunnel/spark/JdbcConfigs.scala:
> + * 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
is JdbcConfigs.scala will be shared by source and sink? if not, maybe can move to org.apache.seatunnel.spark.sink package.
In seatunnel-connectors/seatunnel-connector-spark-jdbc/src/main/scala/org/apache/seatunnel/spark/sink/Jdbc.scala:
>
class Jdbc extends SparkBatchSink {
+ private val LOG: Logger = LoggerFactory.getLogger(classOf[Jdbc])
+
+ private var jdbcConfigs: JdbcConfigs = _
+
+ private def getConnection(): Connection = {
+ Class.forName(jdbcConfigs.driver)
+ val connection = DriverManager.getConnection(jdbcConfigs.url, jdbcConfigs.user, jdbcConfigs.password)
It seems that connection will be created about 3 times at worst, can connection be reused in the same job?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you authored the thread.Message ID: ***@***.***>
--
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] zhaomin1423 commented on a change in pull request #1390: [Feature][spark-connector-jdbc] add pre sql and post sql #1386
Posted by GitBox <gi...@apache.org>.
zhaomin1423 commented on a change in pull request #1390:
URL: https://github.com/apache/incubator-seatunnel/pull/1390#discussion_r819201091
##########
File path: seatunnel-connectors/seatunnel-connector-spark-jdbc/src/main/scala/org/apache/seatunnel/spark/JdbcConfigs.scala
##########
@@ -0,0 +1,81 @@
+/*
+ * 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
Review comment:
It will be used later, I will refactor jdbc connector to add some new features. For example, read data from multiple databases and multiple tables, rate limit, add metrics, etc.
--
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] zhaomin1423 commented on pull request #1390: [Feature][spark-connector-jdbc] add pre sql and post sql #1386
Posted by GitBox <gi...@apache.org>.
zhaomin1423 commented on pull request #1390:
URL: https://github.com/apache/incubator-seatunnel/pull/1390#issuecomment-1060665027
Hi, @CalvinKirs @wuchunfu @asdf2014 Could you like to help me review it. I will do more work based on it.
--
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] zhaomin1423 commented on pull request #1390: [Feature][spark-connector-jdbc] add pre sql and post sql #1386
Posted by GitBox <gi...@apache.org>.
zhaomin1423 commented on pull request #1390:
URL: https://github.com/apache/incubator-seatunnel/pull/1390#issuecomment-1060718381
> Hi @zhaomin1423, would you please merge the dev branch into this PR to pass the CI, thanks
Thanks, I done it.
--
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] zhaomin1423 removed a comment on pull request #1390: [Feature][spark-connector-jdbc] add pre sql and post sql #1386
Posted by GitBox <gi...@apache.org>.
zhaomin1423 removed a comment on pull request #1390:
URL: https://github.com/apache/incubator-seatunnel/pull/1390#issuecomment-1058742890
> But you need to fix the code style of your code, we have scala-style plugin, you can run it locally
I run the command 'mvn checkstyle:checkstyle', it has an error.
<img width="1642" alt="image" src="https://user-images.githubusercontent.com/49054376/156682980-372777d9-27b5-49ae-9a30-b58fcc65fd39.png">
--
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] zhaomin1423 commented on pull request #1390: [Feature][spark-connector-jdbc] add pre sql and post sql #1386
Posted by GitBox <gi...@apache.org>.
zhaomin1423 commented on pull request #1390:
URL: https://github.com/apache/incubator-seatunnel/pull/1390#issuecomment-1058742890
> But you need to fix the code style of your code, we have scala-style plugin, you can run it locally
I run the command 'mvn checkstyle:checkstyle', it has an error.
<img width="1642" alt="image" src="https://user-images.githubusercontent.com/49054376/156682980-372777d9-27b5-49ae-9a30-b58fcc65fd39.png">
--
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 #1390: [Feature][spark-connector-jdbc] add pre sql and post sql #1386
Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on pull request #1390:
URL: https://github.com/apache/incubator-seatunnel/pull/1390#issuecomment-1058291616
But you need to fix the code style of your code, we have scala-style plugin, you can run it locally
--
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] zhaomin1423 commented on pull request #1390: [Feature][spark-connector-jdbc] add pre sql and post sql #1386
Posted by GitBox <gi...@apache.org>.
zhaomin1423 commented on pull request #1390:
URL: https://github.com/apache/incubator-seatunnel/pull/1390#issuecomment-1058820755
> But you need to fix the code style of your code, we have scala-style plugin, you can run it locally
Where is the checkStyle result? I am confused.
--
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] zhaomin1423 edited a comment on pull request #1390: [Feature][spark-connector-jdbc] add pre sql and post sql #1386
Posted by GitBox <gi...@apache.org>.
zhaomin1423 edited a comment on pull request #1390:
URL: https://github.com/apache/incubator-seatunnel/pull/1390#issuecomment-1058254321
Hi，Could you like to help me review it? Also, the license checker were not successful. How should I do?
@CalvinKirs @asdf2014 @wuchunfu
--
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] yx91490 commented on a change in pull request #1390: [Feature][spark-connector-jdbc] add pre sql and post sql #1386
Posted by GitBox <gi...@apache.org>.
yx91490 commented on a change in pull request #1390:
URL: https://github.com/apache/incubator-seatunnel/pull/1390#discussion_r819181327
##########
File path: seatunnel-connectors/seatunnel-connector-spark-jdbc/src/main/scala/org/apache/seatunnel/spark/JdbcConfigs.scala
##########
@@ -0,0 +1,81 @@
+/*
+ * 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
Review comment:
is JdbcConfigs.scala will be shared by source and sink? if not, maybe can move to org.apache.seatunnel.spark.sink package.
##########
File path: seatunnel-connectors/seatunnel-connector-spark-jdbc/src/main/scala/org/apache/seatunnel/spark/sink/Jdbc.scala
##########
@@ -16,47 +16,98 @@
*/
package org.apache.seatunnel.spark.sink
-import scala.collection.JavaConversions._
-
+import org.apache.commons.collections.CollectionUtils
import org.apache.seatunnel.common.config.CheckConfigUtil.checkAllExists
import org.apache.seatunnel.common.config.CheckResult
import org.apache.seatunnel.shade.com.typesafe.config.ConfigFactory
-import org.apache.seatunnel.spark.SparkEnvironment
+import org.apache.seatunnel.spark.{JdbcConfigs, SparkEnvironment}
import org.apache.seatunnel.spark.batch.SparkBatchSink
-import org.apache.spark.sql.{Dataset, Row}
import org.apache.spark.sql.execution.datasources.jdbc2.JDBCSaveMode
+import org.apache.spark.sql.{Dataset, Row}
+import org.slf4j.{Logger, LoggerFactory}
+
+import java.sql.{Connection, DriverManager, Statement}
+import scala.collection.JavaConversions._
class Jdbc extends SparkBatchSink {
+ private val LOG: Logger = LoggerFactory.getLogger(classOf[Jdbc])
+
+ private var jdbcConfigs: JdbcConfigs = _
+
+ private def getConnection(): Connection = {
+ Class.forName(jdbcConfigs.driver)
+ val connection = DriverManager.getConnection(jdbcConfigs.url, jdbcConfigs.user, jdbcConfigs.password)
Review comment:
It seems that connection will be created about 3 times at worst, can connection be reused in the same job?
--
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] zhaomin1423 commented on pull request #1390: [Feature][spark-connector-jdbc] add pre sql and post sql #1386
Posted by GitBox <gi...@apache.org>.
zhaomin1423 commented on pull request #1390:
URL: https://github.com/apache/incubator-seatunnel/pull/1390#issuecomment-1058273157
> I'll check it out later, there is a problem with the license check, but you don‘t added a new Jar, you can ignore this.
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