leekeiabstraction commented on code in PR #188:
URL: 
https://github.com/apache/flink-connector-aws/pull/188#discussion_r2042250368


##########
flink-connector-aws/flink-connector-aws-kinesis-streams/src/main/java/org/apache/flink/connector/kinesis/source/reader/KinesisShardSplitReaderBase.java:
##########
@@ -50,10 +49,10 @@
 /** Base implementation of the SplitReader for reading from 
KinesisShardSplits. */
 @Internal
 public abstract class KinesisShardSplitReaderBase
-        implements SplitReader<Record, KinesisShardSplit> {
+        implements SplitReader<KinesisClientRecord, KinesisShardSplit> {

Review Comment:
   @Lzgpom Can you elaborate on what the user experience would be with the 
current changes in PR? Will it break compilation for existing apps that already 
depend on KinesisSource without further code changes?



##########
flink-connector-aws/flink-connector-aws-kinesis-streams/pom.xml:
##########
@@ -33,6 +33,14 @@ under the License.
     <name>Flink : Connectors : AWS : Amazon Kinesis Data Streams Connector 
v2</name>
     <packaging>jar</packaging>
 
+    <repositories>
+        <!-- used for the kinesis aggregator dependency since it is not 
available in maven central -->
+        <repository>
+            <id>jitpack.io</id>
+            <url>https://jitpack.io</url>
+        </repository>

Review Comment:
   Would it be better to work with the owner of the repo to get this published 
instead?



##########
flink-connector-aws/flink-connector-aws-kinesis-streams/src/main/java/org/apache/flink/connector/kinesis/source/serialization/KinesisDeserializationSchema.java:
##########
@@ -60,7 +60,7 @@ default void open(DeserializationSchema.InitializationContext 
context) throws Ex
      * @param output the identifier of the shard the record was sent to
      * @throws IOException exception when deserializing record
      */
-    void deserialize(Record record, String stream, String shardId, 
Collector<T> output)
+    void deserialize(KinesisClientRecord record, String stream, String 
shardId, Collector<T> output)

Review Comment:
   +1
   
   @Lzgpom Have we considered alternatives to changing the interface? I can see 
that Hong suggested wrapping internally. Have we considered alternatives such 
as having a separate KinesisClientRecordDeserializationSchema?



-- 
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-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to