[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
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[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. I'll merge this 
after my local Travis build passes.


---
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
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[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 | Streaming | Description
| `CsvTableSouce` | `flink-table` | Y | Y | ...
| `Kafka08JsonTableSource` | `flink-connector-kafka-0.8` | N | Y | ...



---
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
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[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 think we can merge this 
now?


---
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
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[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` interface that provides all required 
methods. A `StreamTableSource` can then implement this interface to support 
event-time. I will open a JIRA for 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
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[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 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
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---