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/05/20 02:27:59 UTC
[GitHub] [incubator-seatunnel] legendtkl opened a new pull request, #1931: [Feature][Flink-SQL-connector] add flink sql connector hbase 1.2&2.2 and docs
legendtkl opened a new pull request, #1931:
URL: https://github.com/apache/incubator-seatunnel/pull/1931
## Purpose of this pull request
issue ref: https://github.com/apache/incubator-seatunnel/issues/1930
## Check list
* [ ] Code changed are covered with tests, or it does not need tests for reason:
* [ ] If any new Jar binary package adding in your PR, please add License Notice according
[New License Guide](https://github.com/apache/incubator-seatunnel/blob/dev/docs/en/contribution/new-license.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] legendtkl commented on pull request #1931: [Feature][Flink-SQL-connector] add flink sql connector hbase 1.2&2.2 and docs
Posted by GitBox <gi...@apache.org>.
legendtkl commented on PR #1931:
URL: https://github.com/apache/incubator-seatunnel/pull/1931#issuecomment-1133558173
PR to be UPDATED.
--
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] legendtkl closed pull request #1931: [Feature][Flink-SQL-connector] add flink sql connector hbase 1.2&2.2 and docs
Posted by GitBox <gi...@apache.org>.
legendtkl closed pull request #1931: [Feature][Flink-SQL-connector] add flink sql connector hbase 1.2&2.2 and docs
URL: https://github.com/apache/incubator-seatunnel/pull/1931
--
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] EricJoy2048 commented on pull request #1931: [Feature][Flink-SQL-connector] add flink sql connector hbase 1.2&2.2 and docs
Posted by GitBox <gi...@apache.org>.
EricJoy2048 commented on PR #1931:
URL: https://github.com/apache/incubator-seatunnel/pull/1931#issuecomment-1263376686
Hi, what's news about 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] CalvinKirs commented on a diff in pull request #1931: [Feature][Flink-SQL-connector] add flink sql connector hbase 1.2&2.2 and docs
Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on code in PR #1931:
URL: https://github.com/apache/incubator-seatunnel/pull/1931#discussion_r878740965
##########
seatunnel-connectors/seatunnel-connectors-flink-sql/flink-sql-connector-hbase-2.2/pom.xml:
##########
@@ -0,0 +1,35 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ 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.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <parent>
+ <artifactId>seatunnel-connectors-flink-sql</artifactId>
+ <groupId>org.apache.seatunnel</groupId>
+ <version>${revision}</version>
+ </parent>
+ <modelVersion>4.0.0</modelVersion>
+
+ <artifactId>flink-sql-connector-hbase-2.2</artifactId>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.flink</groupId>
+ <artifactId>flink-connector-hbase-2.2_${scala.binary.version}</artifactId>
+ <version>${flink.version}</version>
+ </dependency>
Review Comment:
the version should statement in root pom(dependency-management)
--
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] legendtkl commented on pull request #1931: [Feature][Flink-SQL-connector] add flink sql connector hbase 1.2&2.2 and docs
Posted by GitBox <gi...@apache.org>.
legendtkl commented on PR #1931:
URL: https://github.com/apache/incubator-seatunnel/pull/1931#issuecomment-1133821152
> > Hi, @ruanwenjun and @BenJFan , I'm afraid the git workflow might need some improvement.
> > I tried the failed command in my machine (MacOS), it works.
> > I think the failure might caused by maven concurrency building '-T'
> > ```shell
> > ./mvnw -T 2C -B clean verify -Dmaven.test.skip=false -Dcheckstyle.skip=true -Dscalastyle.skip=true -Dlicense.skipAddThirdParty=true --no-snapshot-updates
> > ```
> >
> > I suggest to modify it with no concurrency thread build.
> > What do you thinks?
>
> What do you seem to be missing? This CI will list the newly added dependencies,
UT failed with this message "Error: Failed to execute goal org.apache.maven.plugins:maven-assembly-plugin:3.3.0:single (bin) on project seatunnel-dist: Execution bin of goal org.apache.maven.plugins:maven-assembly-plugin:3.3.0:single failed: basedir D:\a\incubator-seatunnel\incubator-seatunnel\seatunnel-dist\..\seatunnel-connectors\seatunnel-connectors-flink-sql-dist\target\lib does not exist -> [Help 1]"
Just as I comment above, the maven concurrency build with `-T C2` might cause this problem. I run the same command on my laptop, it works.
To verify this, I submit a new commit to remove the `-T C2` in git worklow, now the step in CI passed.
So I think we need to update the Github CI workflow (already present in this PR) to avoid the misjudgement. Or other improvement of github CI works is ok.
cc @CalvinKirs @ruanwenjun
--
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] legendtkl commented on pull request #1931: [Feature][Flink-SQL-connector] add flink sql connector hbase 1.2&2.2 and docs
Posted by GitBox <gi...@apache.org>.
legendtkl commented on PR #1931:
URL: https://github.com/apache/incubator-seatunnel/pull/1931#issuecomment-1139230206
> @ruanwenjun @CalvinKirs Do this PR can merge?
Hold it now. cc @BenJFan
--
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] ruanwenjun commented on pull request #1931: [Feature][Flink-SQL-connector] add flink sql connector hbase 1.2&2.2 and docs
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on PR #1931:
URL: https://github.com/apache/incubator-seatunnel/pull/1931#issuecomment-1133798087
> Hi, @ruanwenjun and @BenJFan , I'm afraid the git workflow might need some improvement.
>
> I tried the failed command in my machine (MacOS), it works.
>
> I think the failure might caused by maven concurrency building '-T'
>
> ```shell
> ./mvnw -T 2C -B clean verify -Dmaven.test.skip=false -Dcheckstyle.skip=true -Dscalastyle.skip=true -Dlicense.skipAddThirdParty=true --no-snapshot-updates
> ```
>
> I suggest to modify it with no concurrency thread build.
>
> What do you thinks?
Yeah, if you may get error if you use -T 2C to build in your laptop. But I don't think we need to remove the concurrency setting, since this is used for GitHub CI, you can just remove this when you build on your laptop.
--
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] legendtkl commented on pull request #1931: [Feature][Flink-SQL-connector] add flink sql connector hbase 1.2&2.2 and docs
Posted by GitBox <gi...@apache.org>.
legendtkl commented on PR #1931:
URL: https://github.com/apache/incubator-seatunnel/pull/1931#issuecomment-1133638902
Hi, @ruanwenjun and @BenJFan , I'm afraid the git workflow might need some improvement.
I tried the failed command in my machine (MacOS), it works.
I think the failure might caused by maven concurrency building '-T'
```bash
./mvnw -T 2C -B clean verify -Dmaven.test.skip=false -Dcheckstyle.skip=true -Dscalastyle.skip=true -Dlicense.skipAddThirdParty=true --no-snapshot-updates
```
I suggest to modify it with no concurrency thread build.
What do you thinks?
--
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] BenJFan commented on pull request #1931: [Feature][Flink-SQL-connector] add flink sql connector hbase 1.2&2.2 and docs
Posted by GitBox <gi...@apache.org>.
BenJFan commented on PR #1931:
URL: https://github.com/apache/incubator-seatunnel/pull/1931#issuecomment-1133520884
Seem like you should add license in known-dependencies.txt
--
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 #1931: [Feature][Flink-SQL-connector] add flink sql connector hbase 1.2&2.2 and docs
Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on PR #1931:
URL: https://github.com/apache/incubator-seatunnel/pull/1931#issuecomment-1133723672
> Hi, @ruanwenjun and @BenJFan , I'm afraid the git workflow might need some improvement.
>
> I tried the failed command in my machine (MacOS), it works.
>
> I think the failure might caused by maven concurrency building '-T'
>
> ```shell
> ./mvnw -T 2C -B clean verify -Dmaven.test.skip=false -Dcheckstyle.skip=true -Dscalastyle.skip=true -Dlicense.skipAddThirdParty=true --no-snapshot-updates
> ```
>
> I suggest to modify it with no concurrency thread build.
>
> What do you thinks?
What do you seem to be missing? This CI will list the newly added dependencies,
--
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] BenJFan commented on pull request #1931: [Feature][Flink-SQL-connector] add flink sql connector hbase 1.2&2.2 and docs
Posted by GitBox <gi...@apache.org>.
BenJFan commented on PR #1931:
URL: https://github.com/apache/incubator-seatunnel/pull/1931#issuecomment-1139225126
@ruanwenjun @CalvinKirs Do this PR can merge?
--
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 a diff in pull request #1931: [Feature][Flink-SQL-connector] add flink sql connector hbase 1.2&2.2 and docs
Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on code in PR #1931:
URL: https://github.com/apache/incubator-seatunnel/pull/1931#discussion_r883251271
##########
tools/dependencies/known-dependencies.txt:
##########
@@ -232,36 +236,50 @@ hadoop-yarn-common-2.7.7.jar
hadoop-yarn-common-3.0.0.jar
hadoop-yarn-server-common-2.6.5.jar
hbase-annotations-2.0.0.jar
+hbase-client-1.4.3.jar
hbase-client-2.0.0.jar
hbase-client-2.1.0.jar
+hbase-client-2.2.3.jar
+hbase-common-1.4.3.jar
hbase-common-2.0.0-tests.jar
hbase-common-2.0.0.jar
hbase-common-2.1.0.jar
+hbase-common-2.2.3.jar
hbase-hadoop-compat-2.0.0.jar
hbase-hadoop-compat-2.1.0.jar
+hbase-hadoop-compat-2.2.3.jar
hbase-hadoop2-compat-2.0.0.jar
hbase-hadoop2-compat-2.1.0.jar
+hbase-hadoop2-compat-2.2.3.jar
hbase-http-2.0.0.jar
hbase-http-2.1.0.jar
hbase-mapreduce-2.0.0.jar
hbase-mapreduce-2.1.0.jar
hbase-metrics-2.0.0.jar
hbase-metrics-2.1.0.jar
+hbase-metrics-2.2.3.jar
hbase-metrics-api-2.0.0.jar
hbase-metrics-api-2.1.0.jar
+hbase-metrics-api-2.2.3.jar
hbase-procedure-2.0.0.jar
hbase-procedure-2.1.0.jar
+hbase-protocol-1.4.3.jar
hbase-protocol-2.0.0.jar
hbase-protocol-2.1.0.jar
+hbase-protocol-2.2.3.jar
hbase-protocol-shaded-2.0.0.jar
hbase-protocol-shaded-2.1.0.jar
+hbase-protocol-shaded-2.2.3.jar
hbase-replication-2.0.0.jar
hbase-replication-2.1.0.jar
hbase-server-2.0.0.jar
hbase-server-2.1.0.jar
hbase-shaded-miscellaneous-2.1.0.jar
+hbase-shaded-miscellaneous-2.2.1.jar
hbase-shaded-netty-2.1.0.jar
+hbase-shaded-netty-2.2.1.jar
hbase-shaded-protobuf-2.1.0.jar
Review Comment:
Cool, but some NOTICE and License may be missing. Another question, can we unify the version, (maybe not), for example we have multiple versions of hbase.
--
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