[GitHub] [flink] CrynetLogistics commented on pull request #17345: [FLINK-24227][connectors] FLIP-171: Added Kinesis Data Streams Sink i…

2021-10-11 Thread GitBox 


CrynetLogistics commented on pull request #17345:
URL: https://github.com/apache/flink/pull/17345#issuecomment-940378397


   Tests now passing. Will deal with the rest of the comments tomorrow!


-- 
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

[GitHub] [flink] CrynetLogistics commented on pull request #17345: [FLINK-24227][connectors] FLIP-171: Added Kinesis Data Streams Sink i…

2021-10-04 Thread GitBox 


CrynetLogistics commented on pull request #17345:
URL: https://github.com/apache/flink/pull/17345#issuecomment-933406220


   Comment From Nuno
   `KinesisDataStreamsSinkConfig`
   Should we change the variable `DEFAULT_MAX_TimeInBufferMS` to 
`DEFAULT_MAX_TIME_IN_BUFFER_MS`?
   `KinesisDataStreamsSinkITCase`
   Would it be possible to move the following code snippet from runScenario to 
its own method (like prepareStream )? This method seems a bit large at the 
moment.
   ```kinesisClient
   .createStream(
   CreateStreamRequest.builder()
   .streamName(testStreamName)
   .shardCount(1)
   .build())
   .get();
   
   DescribeStreamResponse describeStream =
   kinesisClient
   .describeStream(
   
DescribeStreamRequest.builder().streamName(testStreamName).build())
   .get();
   
   while (describeStream.streamDescription().streamStatus() != 
StreamStatus.ACTIVE) {
   describeStream =
   kinesisClient
   .describeStream(
   DescribeStreamRequest.builder()
   .streamName(testStreamName)
   .build())
   .get();
   }```
   In the previous while loop, do we actually need to be creating the 
describeStream object every time? Would it be possible to have an object with 
the result of 
`kinesisClient.describeStream(escribeStreamRequest.builder().streamName(testStreamName).build())`
 and call `.get().streamDescription().streamStatus()` when want to query the 
status? Something like:
   `while (.get().streamDescription().streamStatus() != 
StreamStatus.ACTIVE);`


-- 
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

[GitHub] [flink] CrynetLogistics commented on pull request #17345: [FLINK-24227][connectors] FLIP-171: Added Kinesis Data Streams Sink i…

2021-09-23 Thread GitBox 


CrynetLogistics commented on pull request #17345:
URL: https://github.com/apache/flink/pull/17345#issuecomment-926092248


   For the reviewer, please note, **reflection** is used in 
`KinesisDataStreamsSinkITCase` to access a `private static final` client in 
`KinesisDataStreamsSinkWriter`. The use of reflection is generally frowned 
upon, however, I believe this is one of the few situations where the use of 
reflection is appropriate. Any suggestions would be very warmly welcomed. 


-- 
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