[GitHub] flink pull request: [FLINK-3058] Add support for Kafka 0.9.0.0

2016-01-20 Thread rmetzger
Github user rmetzger commented on the pull request:

https://github.com/apache/flink/pull/1489#issuecomment-173243638
  
I addressed all concerns and rebased to master.

Once the tests have passed, I'll merge the change.


---
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 pull request: [FLINK-3058] Add support for Kafka 0.9.0.0

2016-01-20 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/flink/pull/1489


---
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 pull request: [FLINK-3058] Add support for Kafka 0.9.0.0

2016-01-19 Thread StephanEwen
Github user StephanEwen commented on the pull request:

https://github.com/apache/flink/pull/1489#issuecomment-172957450
  
I would leave a deprecated producer, just to make user's life easier...


---
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 pull request: [FLINK-3058] Add support for Kafka 0.9.0.0

2016-01-19 Thread rmetzger
Github user rmetzger commented on the pull request:

https://github.com/apache/flink/pull/1489#issuecomment-172932775
  
Thank you all for the comments. I renamed the Consumers again to include 
the version, I added deprecated 081 and 092 consumers.
Do you think we should also add a deprecated FlinkKafkaProducer? (The 
producers now have a 08 and 09 suffix as well).
I also worked on the test stability. Lets see what travis says.

If there are no further comments, I'll soon rebase and merge the pull 
request.


---
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 pull request: [FLINK-3058] Add support for Kafka 0.9.0.0

2016-01-15 Thread aljoscha
Github user aljoscha commented on the pull request:

https://github.com/apache/flink/pull/1489#issuecomment-171933036
  
I'm not sure because I don't know everything about it but why not rename 
`KafkaServerProvider` to something that more clearly says what it is, like 
`KafkaTestEnvironment`. 


---
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 pull request: [FLINK-3058] Add support for Kafka 0.9.0.0

2016-01-15 Thread aljoscha
Github user aljoscha commented on the pull request:

https://github.com/apache/flink/pull/1489#issuecomment-171933355
  
Are you sure it is a good idea to give the same name to the 
producers/consumers for 0.8 and 0.9? Because now we have two 
`org.apache.flink.streaming.connectors.kafka.FlinkKafkaConsumer` (and Producer) 
and what that refers to depends on what jar files are loaded first, correct?


---
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 pull request: [FLINK-3058] Add support for Kafka 0.9.0.0

2016-01-15 Thread rmetzger
Github user rmetzger commented on the pull request:

https://github.com/apache/flink/pull/1489#issuecomment-171903718
  
Okay, I'll rename the consumers and producers to include the version. 
(FlinkKafkaConsumer08 and so on.)


---
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 pull request: [FLINK-3058] Add support for Kafka 0.9.0.0

2016-01-14 Thread StephanEwen
Github user StephanEwen commented on the pull request:

https://github.com/apache/flink/pull/1489#issuecomment-171741270
  
This PR changes the name of the KafkaConsumer classes. Both of them are now 
called `FlinkKafkaConsumer` in the exact same namespace, and only differ in 
their Maven project.

I think that is dangerous (classes with exact same qualified name). We have 
seem many cases where people work with unclean dependencies, which would result 
in a name clash if both dependencies are accidentally included.
The end result being that the wrong class is used, the connector does not 
work, and is non trivial to recognize that for users.

I would vote for the following:
  - Put a qualifier either in the class name or the package: 
`.connectors.kafka.FlinkKafkaConsumer08` or 
`.connectors.kafka08.FlinkKafkaConsumer`
  - Keep the current 0.8 classes for compatibility and deprecate 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 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 pull request: [FLINK-3058] Add support for Kafka 0.9.0.0

2016-01-14 Thread StephanEwen
Github user StephanEwen commented on the pull request:

https://github.com/apache/flink/pull/1489#issuecomment-171736931
  
We have not looked into how Kafka uses Kerberos, yet, so a ticket would be 
good.


---
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 pull request: [FLINK-3058] Add support for Kafka 0.9.0.0

2016-01-14 Thread nielsbasjes
Github user nielsbasjes commented on the pull request:

https://github.com/apache/flink/pull/1489#issuecomment-171665915
  
I read that Kafka 0.9 supports Kerberos authentication (I have not yet 
tried this). Is that supported in this first release or should I open a Jira 
ticket for that?


---
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 pull request: [FLINK-3058] Add support for Kafka 0.9.0.0

2016-01-08 Thread rmetzger
Github user rmetzger commented on the pull request:

https://github.com/apache/flink/pull/1489#issuecomment-170103132
  
There are some build instabilities with the new Kafka 0.9 code. I'll look 
into it soon.


---
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 pull request: [FLINK-3058] Add support for Kafka 0.9.0.0

2016-01-08 Thread tillrohrmann
Github user tillrohrmann commented on the pull request:

https://github.com/apache/flink/pull/1489#issuecomment-170044651
  
Test cases `Kafka09ITCase.testMultipleSourcesOnePartition` and 
`Kafka08ITCase.testOffsetInZookeeper` are failing in Travis build.


---
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 pull request: [FLINK-3058] Add support for Kafka 0.9.0.0

2016-01-06 Thread rmetzger
GitHub user rmetzger opened a pull request:

https://github.com/apache/flink/pull/1489

[FLINK-3058] Add support for Kafka 0.9.0.0

For adding Kafka 0.9.0.0 support, this commit changes the following:
- Split up of the kafka connector into a 
flink-connector-kafka-(base|0.9|0.8) with different dependencies
- The base package contains common test cases and implementations (for 
example the producer for 0.9 and 0.8 relies on exactly the same code)
- the 0.8 package contains a kafka connector implementation against the 
SimpleConsumer (low level) API of Kafka 0.8. There are some additional tests 
for the ZK offset committing
- The 0.9 package relies on the new Consumer API of Kafka 0.9.0.0
- Support for metrics for all producers and the 0.9 consumer through 
Flink's accumulators.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/rmetzger/flink 
flink3058-second-rebased-rebased

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/flink/pull/1489.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1489


commit d1a1659029b246cc164fe3cb274b02d87696e679
Author: Robert Metzger 
Date:   2015-12-16T16:29:42Z

[FLINK-3058] Add support for Kafka 0.9.0.0

For adding Kafka 0.9.0.0 support, this commit changes the following:
- Split up of the kafka connector into a 
flink-connector-kafka-(base|0.9|0.8) with different dependencies
- The base package contains common test cases, classes and implementations 
(the producer for 0.9 and 0.8 relies on exactly the same code)
- the 0.8 package contains a kafka connector implementation against the 
SimpleConsumer (low level) API of Kafka 0.8. There are some additional tests 
for the ZK offset committing
- The 0.9 package relies on the new Consumer API of Kafka 0.9.0.0
- Support for metrics for all producers and the 0.9 consumer through 
Flink's accumulators.




---
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.
---