You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/03/09 09:16:50 UTC
[GitHub] [flink] zentol commented on a change in pull request #18905: [FLINK-25927][connectors] Make flink-connector-base dependency usage consistent across all connectors
zentol commented on a change in pull request #18905:
URL: https://github.com/apache/flink/pull/18905#discussion_r822417080
##########
File path: flink-end-to-end-tests/flink-quickstart-test/pom.xml
##########
@@ -37,6 +37,11 @@ under the License.
<artifactId>flink-quickstart-java</artifactId>
<version>${project.version}</version>
</dependency>
+ <dependency>
+ <groupId>org.apache.flink</groupId>
+ <artifactId>flink-connector-base</artifactId>
Review comment:
shouldn't be necessary because it is bundled in the connector and the test itself doesn't require the connector to be on the classpath
##########
File path: flink-formats/flink-parquet/pom.xml
##########
@@ -308,6 +314,33 @@ under the License.
</execution>
</executions>
</plugin>
+
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-shade-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>shade-flink</id>
+ <phase>package</phase>
+ <goals>
+ <goal>shade</goal>
+ </goals>
+ <configuration>
+ <artifactSet>
+ <includes>
+ <include>org.apache.flink:flink-connector-base</include>
+ </includes>
+ </artifactSet>
+ <relocations>
+ <relocation>
+ <pattern>org.apache.flink.connector.base</pattern>
+ <shadedPattern>org.apache.flink.connector.kinesis.shaded.org.apache.flink.connector.base</shadedPattern>
Review comment:
pattern is incorrect
##########
File path: flink-connectors/flink-sql-connector-kafka/pom.xml
##########
@@ -40,6 +40,12 @@ under the License.
<groupId>org.apache.flink</groupId>
<artifactId>flink-connector-kafka</artifactId>
<version>${project.version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>org.apache.flink</groupId>
+ <artifactId>flink-connector-base</artifactId>
Review comment:
such exclusions should be unnecessary because the shade-plugin removes dependencies that are bundled.
##########
File path: flink-connectors/flink-connector-kafka/pom.xml
##########
@@ -288,6 +288,33 @@ under the License.
<argLine>-Xms256m -Xmx2048m -Dmvn.forkNumber=${surefire.forkNumber} -XX:-UseGCOverheadLimit -Duser.country=US -Duser.language=en</argLine>
</configuration>
</plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-shade-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>shade-flink</id>
+ <phase>package</phase>
+ <goals>
+ <goal>shade</goal>
+ </goals>
+ <configuration>
+ <artifactSet>
+ <includes>
+ <include>org.apache.flink:flink-connector-base:*</include>
Review comment:
```suggestion
<include>org.apache.flink:flink-connector-base</include>
```
##########
File path: flink-formats/flink-json/pom.xml
##########
@@ -77,6 +77,13 @@ under the License.
<!-- test dependencies -->
+ <dependency>
+ <groupId>org.apache.flink</groupId>
+ <artifactId>flink-connector-base</artifactId>
Review comment:
why is this required? (same question for other formats)
##########
File path: flink-connectors/flink-sql-connector-kafka/pom.xml
##########
@@ -82,6 +88,10 @@ under the License.
<pattern>org.apache.kafka</pattern>
<shadedPattern>org.apache.flink.kafka.shaded.org.apache.kafka</shadedPattern>
</relocation>
+ <relocation>
+ <pattern>org.apache.flink.connector.base</pattern>
+ <shadedPattern>org.apache.flink.connector.kafka.sql.shaded.org.apache.flink.connector.base</shadedPattern>
Review comment:
should be unnecessary since the connector itself bundles the base module. Also, connector-base can be removed from the artifactSet.
##########
File path: flink-formats/flink-orc/pom.xml
##########
@@ -204,6 +210,33 @@ under the License.
</execution>
</executions>
</plugin>
+
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-shade-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>shade-flink</id>
+ <phase>package</phase>
+ <goals>
+ <goal>shade</goal>
+ </goals>
+ <configuration>
+ <artifactSet>
+ <includes>
+ <include>org.apache.flink:flink-connector-base</include>
+ </includes>
+ </artifactSet>
+ <relocations>
+ <relocation>
+ <pattern>org.apache.flink.connector.base</pattern>
+ <shadedPattern>org.apache.flink.connector.kinesis.shaded.org.apache.flink.connector.base</shadedPattern>
Review comment:
pattern is incorrect
##########
File path: tools/ci/shade.sh
##########
@@ -173,3 +173,62 @@ check_shaded_artifacts_connector_elasticsearch() {
return 0
}
+
+check_one_per_package() {
+ read foo
+ if [ $foo -gt 1 ]
+ then
+ echo "ERROR - CHECK FAILED: $1 is shaded multiple times!"
+ exit 1
+ else
+ echo "OK"
+ fi
+}
+
+check_relocated() {
+ read foo
+ if [ $foo -ne 0 ]
+ then
+ echo "ERROR - CHECK FAILED: found $1 classses that where not relocated!"
Review comment:
```suggestion
echo "ERROR - CHECK FAILED: found $1 classes that where not relocated!"
```
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org