dannycranmer commented on code in PR #9:
URL: 
https://github.com/apache/flink-connector-dynamodb/pull/9#discussion_r995487077


##########
flink-connector-dynamodb/pom.xml:
##########
@@ -33,6 +33,15 @@ under the License.
     <artifactId>flink-connector-dynamodb</artifactId>
     <name>Flink : Connectors : Amazon DynamoDB</name>
 
+       <dependencies>
+               <dependency>
+                       <groupId>org.apache.flink</groupId>
+                       <artifactId>flink-test-utils</artifactId>
+                       <version>1.16-SNAPSHOT</version>

Review Comment:
   Why are we depending on a snapshot version and not the parent Flink version? 
Can this dependency not be managed from the dynamodb parent pom?



##########
flink-sql-connector-dynamodb/pom.xml:
##########
@@ -51,6 +51,13 @@ under the License.
                        <artifactId>flink-connector-dynamodb</artifactId>
                        <version>${project.version}</version>
                </dependency>
+
+               <dependency>
+                       <groupId>org.apache.flink</groupId>
+                       <artifactId>flink-test-utils</artifactId>
+                       <version>1.16-SNAPSHOT</version>

Review Comment:
   Same as above



##########
flink-connector-dynamodb/src/test/java/org/apache/flink/connectors/dynamodb/PackagingITCase.java:
##########
@@ -0,0 +1,19 @@
+package org.apache.flink.connectors.dynamodb;
+
+import org.apache.flink.packaging.PackagingTestUtils;
+import org.apache.flink.test.resources.ResourceTestUtils;
+
+import org.junit.jupiter.api.Test;
+
+import java.nio.file.Path;
+import java.util.Arrays;
+
+/** Test to verify contents of packaged jar. */
+public class PackagingITCase {
+    @Test
+    void testPackaging() throws Exception {
+        final Path jar = 
ResourceTestUtils.getResource(".*/flink-connector-dynamodb-[^/]*\\.jar");
+
+        PackagingTestUtils.assertJarContainsOnlyFilesMatching(jar, 
Arrays.asList("META-INF/"));

Review Comment:
   This will break the build for the main code contribution I assume. Can we 
relax this check to support the intended result? We expect class file 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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to