[GitHub] flink issue #3112: [FLINK-4988] [elasticsearch] Add Elasticsearch 5.x Connec...

2017-02-07 Thread tzulitai
Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/3112 Merging to `master` .. --- 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

[GitHub] flink issue #3112: [FLINK-4988] [elasticsearch] Add Elasticsearch 5.x Connec...

2017-02-07 Thread tzulitai
Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/3112 Thank you for the review! I'll address the missing semicolons and merge this today. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well.

[GitHub] flink issue #3112: [FLINK-4988] [elasticsearch] Add Elasticsearch 5.x Connec...

2017-02-07 Thread rmetzger
Github user rmetzger commented on the issue: https://github.com/apache/flink/pull/3112 +1 to merge. I've tried the ES2 connector (just to check one of the connectors) and it worked well. --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] flink issue #3112: [FLINK-4988] [elasticsearch] Add Elasticsearch 5.x Connec...

2017-01-31 Thread tzulitai
Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/3112 Hi @fpompermaier! Thanks for bringing it up. However, as Robert suggested, lets not include that as this PR. But I'll like to discuss with you on the matter on the corresponding JIRA concurrently

[GitHub] flink issue #3112: [FLINK-4988] [elasticsearch] Add Elasticsearch 5.x Connec...

2017-01-31 Thread fpompermaier
Github user fpompermaier commented on the issue: https://github.com/apache/flink/pull/3112 Hi @tzulitai, congrats for the great work! For my use case it is important also to be resilient to malformed documents (https://issues.apache.org/jira/browse/FLINK-5353). Do you think you could

[GitHub] flink issue #3112: [FLINK-4988] [elasticsearch] Add Elasticsearch 5.x Connec...

2017-01-31 Thread tzulitai
Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/3112 I think the PR is in a stable state for a final review now. I'll keep the PR as is until another review, sorry for jumping around as I was trying options for more future change proof. --- If your

[GitHub] flink issue #3112: [FLINK-4988] [elasticsearch] Add Elasticsearch 5.x Connec...

2017-01-31 Thread tzulitai
Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/3112 While rebasing #2861 on the restructured ES connectors in this PR, I jumped around different options on whether or not the `BulkProcessor` build process should be abstracted. In the end, I think it

[GitHub] flink issue #3112: [FLINK-4988] [elasticsearch] Add Elasticsearch 5.x Connec...

2017-01-29 Thread tzulitai
Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/3112 Thank you for your comments @mikedias, I will address them. --- 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

[GitHub] flink issue #3112: [FLINK-4988] [elasticsearch] Add Elasticsearch 5.x Connec...

2017-01-24 Thread tzulitai
Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/3112 Rebased to resolve conflicts in documentation changes. --- 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

[GitHub] flink issue #3112: [FLINK-4988] [elasticsearch] Add Elasticsearch 5.x Connec...

2017-01-16 Thread tzulitai
Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/3112 @rmetzger The comments are all addressed now, including the last two problems. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If

[GitHub] flink issue #3112: [FLINK-4988] [elasticsearch] Add Elasticsearch 5.x Connec...

2017-01-16 Thread rmetzger
Github user rmetzger commented on the issue: https://github.com/apache/flink/pull/3112 Gordon and I had a quick offline chat about this and decided the following: 1. We'll use the `ServiceResourceTransformer` because that's the correct way of solving the problem 2. There is a

[GitHub] flink issue #3112: [FLINK-4988] [elasticsearch] Add Elasticsearch 5.x Connec...

2017-01-13 Thread tzulitai
Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/3112 Thanks for the pointers. We'll include the profile `include-elasticsearch5` for Java 8 builds only. --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] flink issue #3112: [FLINK-4988] [elasticsearch] Add Elasticsearch 5.x Connec...

2017-01-13 Thread rmetzger
Github user rmetzger commented on the issue: https://github.com/apache/flink/pull/3112 Using maven build profiles, you can probably include the es5 connector in java8 builds only. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] flink issue #3112: [FLINK-4988] [elasticsearch] Add Elasticsearch 5.x Connec...

2017-01-13 Thread tzulitai
Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/3112 @rmetzger Ah ... seems like ES 5.x requires at least Java 8. - https://www.elastic.co/guide/en/elasticsearch/reference/master/_installation.html#_installation -

[GitHub] flink issue #3112: [FLINK-4988] [elasticsearch] Add Elasticsearch 5.x Connec...

2017-01-13 Thread rmetzger
Github user rmetzger commented on the issue: https://github.com/apache/flink/pull/3112 Thanks a lot for opening a pull request for this. It looks like some of the tests are failing on travis. Does ES5 support Java 7 ? --- If your project is set up for it, you can reply to this