-
Notifications
You must be signed in to change notification settings - Fork 4.5k
[BEAM-52] Kafka IO #142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BEAM-52] Kafka IO #142
Conversation
|
Forward-porting GoogleCloudPlatform/DataflowJavaSDK#121. Note that the original author is a Googler (hence covered by CLA) and is still attributed to the actual work. For more information, see the considerable review in the original PR. I fully squashed the original PR and fixed some licenses. More work still in progress. |
contrib/examples/kafka/pom.xml
Outdated
|
|
||
| <groupId>com.google.cloud.dataflow</groupId> | ||
| <artifactId>google-cloud-dataflow-java-contrib-kafka-examples</artifactId> | ||
| <name>Google Cloud Dataflow Kafka Examples</name> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can change to use org.apache.beam Maven coordonates ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyway, aligned with the other IOs and current discussion, we can move this in the sdks/java/io module later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
|
Just couple of small comments (about Maven coordonates and module). Great work anyway ! |
|
Thanks @jbonofre ! We'll take these comments into account as we do the cleanups in the TODO list in the PR description. |
|
Looking in to Travis failure. |
.travis.yml
Outdated
| script: | ||
| - travis_retry mvn -B $MAVEN_OVERRIDE install -U | ||
| - travis_retry travis/test_wordcount.sh | ||
| - (cd contrib/kafka && mvn -B $MAVEN_OVERRIDE install -U) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want to test this in travis?
|
@dhalperi , I made the package renaming changes this morning, only to realize you have already done that.. |
|
Thanks Dan! Glad to see KafkaIO is moved out of contrib. |
*) package renaming
*) move it out of contrib to I/O module
*) get it running as part of the normal travis build,
and remove specialization
*) remove examples, to be added in a separate PR.
Either there should be an addition to an existing example to
handle "multiple" streaming sources, or we should have this as an
integration test. No need, however, for an example just to show
how to use an I/O in a pipeline -- that's what Javadoc is for.
|
LGTM @dhalperi. Thanks for making the changes. I agree with your comment on reevaluating if and where example app fits in. |
#! [euphoria-spark] Correctly transfer global state to executed UDFs
…nfigmap remove flinkConfMap from FlinkPipelineOptions
Be sure to do all of the following to help us incorporate your contribution
quickly and easily:
[BEAM-<Jira issue #>] Description of pull requestmvn clean verify. (Even better, enableTravis-CI on your fork and ensure the whole test matrix passes).
number, if there is one.
Individual Contributor License Agreement.
TODO: @rangadi @dhalperi
R: @dpmills @dhalperi