[GitHub] [flink] dawidwys commented on issue #10674: [FLINK-15220][Connector/Kafka][Table] Add startFromTimestamp in KafkaTableSource

2020-01-13 Thread GitBox
dawidwys commented on issue #10674: [FLINK-15220][Connector/Kafka][Table] Add 
startFromTimestamp in KafkaTableSource
URL: https://github.com/apache/flink/pull/10674#issuecomment-574046614
 
 
   Thank you for the update @link3280 . Will merge it later today.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [flink] dawidwys commented on issue #10674: [FLINK-15220][Connector/Kafka][Table] Add startFromTimestamp in KafkaTableSource

2020-01-13 Thread GitBox
dawidwys commented on issue #10674: [FLINK-15220][Connector/Kafka][Table] Add 
startFromTimestamp in KafkaTableSource
URL: https://github.com/apache/flink/pull/10674#issuecomment-573553180
 
 
   @link3280 Generally looks really good. Could you just update the remaining 
languages in the docs? (DDL, YAML, Python)


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [flink] dawidwys commented on issue #10674: [FLINK-15220][Connector/Kafka][Table] Add startFromTimestamp in KafkaTableSource

2020-01-13 Thread GitBox
dawidwys commented on issue #10674: [FLINK-15220][Connector/Kafka][Table] Add 
startFromTimestamp in KafkaTableSource
URL: https://github.com/apache/flink/pull/10674#issuecomment-573549541
 
 
   @wuchong I am not sure if an additional `Z` is such a hassle for users. I 
really think specifying instants should be as explicit as possible. That said I 
think splitting that to a separate issue is a good idea.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [flink] dawidwys commented on issue #10674: [FLINK-15220][Connector/Kafka][Table] Add startFromTimestamp in KafkaTableSource

2020-01-10 Thread GitBox
dawidwys commented on issue #10674: [FLINK-15220][Connector/Kafka][Table] Add 
startFromTimestamp in KafkaTableSource
URL: https://github.com/apache/flink/pull/10674#issuecomment-573030241
 
 
   The changes look mostly good. I have some concerns regarding using a 
`LocalDateTime` as an `Instant`.
   
   To be honest, I don't like that `connector.startup-timestamp` assumes UTC as 
the time zone. I think this will be troublesome. I understand this is a helpful 
feature so I would suggest to expect the user to provide the time-zone 
explicitly. That said rather than parsing a `LocalDateTime` from the 
properties. I would parse an `OffsetDateTime` from the properties. For UTC 
users can pass a date as e.g. `2011-12-03T10:15:30Z'


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services