[GitHub] flink issue #5171: [FLINK-8271][Kinesis connector] upgrade from deprecated c...

2017-12-20 Thread tzulitai
Github user tzulitai commented on the issue:

https://github.com/apache/flink/pull/5171
  
Ah, I see. The `AmazonKinesisClient` class is not deprecated, but the 
constructors are.
Alright, thanks for the clarification. I'll find some time to revisit this 
PR tomorrow.


---


[GitHub] flink issue #5171: [FLINK-8271][Kinesis connector] upgrade from deprecated c...

2017-12-18 Thread bowenli86
Github user bowenli86 commented on the issue:

https://github.com/apache/flink/pull/5171
  
No, those classes have been deprecated in AWS SDK 1.11.171.


---


[GitHub] flink issue #5171: [FLINK-8271][Kinesis connector] upgrade from deprecated c...

2017-12-18 Thread tzulitai
Github user tzulitai commented on the issue:

https://github.com/apache/flink/pull/5171
  
Without the SDK upgrade, the API actually isn't yet deprecated yet, right?
It is only deprecated in a newer SDK version.

If so, I would prefer to not merge this until we actually try to upgrade 
the AWS Java SDK version. Then, this change would be more context relevant.


---


[GitHub] flink issue #5171: [FLINK-8271][Kinesis connector] upgrade from deprecated c...

2017-12-18 Thread tzulitai
Github user tzulitai commented on the issue:

https://github.com/apache/flink/pull/5171
  
@bowenli86 I mean't the actual commit messages, not the PR title / 
description.
The actual commit messages should follow our standard commit message 
formats.


---


[GitHub] flink issue #5171: [FLINK-8271][Kinesis connector] upgrade from deprecated c...

2017-12-17 Thread bowenli86
Github user bowenli86 commented on the issue:

https://github.com/apache/flink/pull/5171
  
A good point for testing this in cluster mode. I don't have a 1.5 cluser so 
only tested in local mode. The upgrade is unnecessary, I'll revert it.

Which commit message in PR submission are you referring to? The "What is 
the purpose of the change ..." part, or the one for each commit?


---


[GitHub] flink issue #5171: [FLINK-8271][Kinesis connector] upgrade from deprecated c...

2017-12-17 Thread tzulitai
Github user tzulitai commented on the issue:

https://github.com/apache/flink/pull/5171
  
Please also keep in mind that it would be a good contribution practice to 
have meaningful commit messages in PR submissions.
Ideally, they should be as you would like them to be when they are merged.


---