[GitHub] flink issue #2069: [FLINK-3872] [table, connector-kafka] Add KafkaJsonTableS...

2016-06-23 Thread uce
Github user uce commented on the issue: https://github.com/apache/flink/pull/2069 Test failure is unrelated, going to merge this. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature

[GitHub] flink issue #2069: [FLINK-3872] [table, connector-kafka] Add KafkaJsonTableS...

2016-06-21 Thread uce
Github user uce commented on the issue: https://github.com/apache/flink/pull/2069 Thanks for the suggestion Fabian. I've added the table to the docs and added an example about the Kafka JSON source. Furthermore, I've added the configuration flag for the missing field failure behaviour

[GitHub] flink issue #2069: [FLINK-3872] [table, connector-kafka] Add KafkaJsonTableS...

2016-06-10 Thread fhueske
Github user fhueske commented on the issue: https://github.com/apache/flink/pull/2069 Hi @uce , the update looks good. I think we should add the new TableSources to the Table API documentation. Maybe adding a table to like to `table.md`? | Class name | Maven dep | BatchSource

[GitHub] flink issue #2069: [FLINK-3872] [table, connector-kafka] Add KafkaJsonTableS...

2016-06-10 Thread uce
Github user uce commented on the issue: https://github.com/apache/flink/pull/2069 Thanks for the review Fabian. I've addressed all comments except the ObjectMapper configuration. The EvenTimeTableSource makes sense. We can make the Kafka source extend from that one later on. Do you th

[GitHub] flink issue #2069: [FLINK-3872] [table, connector-kafka] Add KafkaJsonTableS...

2016-06-10 Thread fhueske
Github user fhueske commented on the issue: https://github.com/apache/flink/pull/2069 Hi @uce, thanks for the PR. Looks really good. I added three comments inline. Regarding the watermarks and timestamp handling, I am thinking about introducing an `EventTimeTableSource` inte

[GitHub] flink issue #2069: [FLINK-3872] [table, connector-kafka] Add KafkaJsonTableS...

2016-06-03 Thread rmetzger
Github user rmetzger commented on the issue: https://github.com/apache/flink/pull/2069 Quickly checked the maven and kafka stuff. Looks good. Very clean, well documented and tested code. Further review for the table api specific parts are still needed. --- If your project is set