Github user rmetzger commented on the issue:
https://github.com/apache/flink/pull/2231
@radekg I've added Kafka 0.10 support to Flink, that's why I closed this
pull request. My change preserved your commit from this pull request. Thank you
for the contribution!
---
If your project i
Github user rmetzger commented on the issue:
https://github.com/apache/flink/pull/2231
@radekg I've opened a PR based on your code:
https://github.com/apache/flink/issues/2369 feel free to review it.
---
If your project is set up for it, you can reply to this email and have your
repl
Github user rmetzger commented on the issue:
https://github.com/apache/flink/pull/2231
Okay, cool. Thank you. I'll probably open a pull request with your and my
changes. I'll let you know so that you can help reviewing it.
---
If your project is set up for it, you can reply to this
Github user radekg commented on the issue:
https://github.com/apache/flink/pull/2231
@rmetzger it's absolutely fine to reuse the code. If I can help in any way,
please let me know.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user rmetzger commented on the issue:
https://github.com/apache/flink/pull/2231
@radekg, are you okay with me using your pull request as a base for adding
Kafka 0.10 to Flink?
I've started changing your code from the PR so that we don't need to copy
so much code: https://gi
Github user rmetzger commented on the issue:
https://github.com/apache/flink/pull/2231
Sorry for joining this discussion late. I've been on vacation.
I also stumbled across the code duplicates. I'll check out the code from
this pull request and see if there's a good way of re-using
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/2231
@radekg One thing that's missing is update to the docs. You can find the
Kafka connector documentation at
`flink/docs/apis/streaming/connectors/kafka.md`. We'lll probably only need to
update the Ma
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/2231
Finished first review of the code.
Let me summarize parts of the Kafka 0.10 API that requires us to have a
separate module:
1. `ConsumerRecord` in 0.10 has a new `ConsumerRecord#ti
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/2231
Sorry for the delay on the review. Reviewing + testing now ...
@radekg Yup the failure is unrelated to the changes here. Some of the tests
are currently a bit flaky.
---
If your project is
Github user radekg commented on the issue:
https://github.com/apache/flink/pull/2231
Tests are failing for random setups on travis. Seems to be something scala
related.
---
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 user tzulitai commented on the issue:
https://github.com/apache/flink/pull/2231
@radekg Thank you for the quick fix. I hope to find time over the weekend
to test + review this, if not than early next week :)
---
If your project is set up for it, you can reply to this email and
Github user radekg commented on the issue:
https://github.com/apache/flink/pull/2231
Travis is going to run.
---
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
Github user radekg commented on the issue:
https://github.com/apache/flink/pull/2231
Thanks, running `verify` again.
---
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 w
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/2231
The errors are due to some of the changes to `AbstractFetcher` in
https://github.com/apache/flink/commit/41f58182289226850b23c61a32f01223485d4775.
Some of the Kafka 0.9 connector code that has chang
Github user radekg commented on the issue:
https://github.com/apache/flink/pull/2231
Merged with `upstream/master` and I'm getting this when running `mvn clean
verify`:
```
[INFO] -
[ERROR] COMPILATION ERROR :
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/2231
Hi @radekg ,
There was a recent change to the connector code to how the Kafka metrics
are reported, so right now the PR has conflicts and can't be built. Would you
like to rebase this PR on the
Github user radekg commented on the issue:
https://github.com/apache/flink/pull/2231
@tzulitai yes, this pr does not deal with 0.10 specific timestamps. It
makes a simple consumer application work.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/2231
@aljoscha Ah right, for the new timestamps we will definitely need a
0.10-specific consumer. I think it makes sense to include a new module then.
This current PR does not add the new 0.10 ti
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2231
I agree with @tzulitai that it would be nice if you could have minimum code
duplication in the long-run but it might not be possible with the current
design of the consumers.
What about the
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/2231
Thank you for the description, @radekg .
I think the problems you mentioned should be solvable by working on the 0.9
connector to be just a bit more general, then users can simply manually u
Github user radekg commented on the issue:
https://github.com/apache/flink/pull/2231
Sure, the problems are the following:
-
https://github.com/apache/flink/pull/2231/commits/06936d7c5acc0897348019161c9ced4596a0a4dd#diff-aba21cf86694f3f2cd85e2e5e9b04972R305
in 0.9, `consumer.
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/2231
Hi @radekg , thank you for opening a PR for this!
From a first look it seems that there isn't much changes to the code of
`flink-connector-kafka-0.9` and this PR. Also, from the original discussi
22 matches
Mail list logo
