Skip to content
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-6207] Added option to publish synthetic data to Kafka topic. #7612

Merged
merged 2 commits into from Jan 30, 2019

Conversation

mwalenia
Copy link
Member

@mwalenia mwalenia commented Jan 24, 2019

SyntheticDataPubSubPublisher now accepts --kafkaBootstrapServerAddress option
which switches it to Kafka publishing mode instead of Google PubSub mode.
Key-Value pairs are published as strings.


Follow this checklist to help us incorporate your contribution quickly and easily:

  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

It will help us expedite review of your Pull Request if you tag someone (e.g. @username) to look at it.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- --- --- --- ---
Java Build Status Build Status Build Status Build Status
Build Status
Build Status
Build Status Build Status Build Status
Python Build Status --- Build Status
Build Status
--- --- --- ---

@mwalenia
Copy link
Member Author

Hi @kkucharc , could you look at it? Thanks!

@mwalenia mwalenia force-pushed the BEAM-6207-add-kafka-to-synth-pubsub branch 2 times, most recently from db59671 to a87a869 Compare January 24, 2019 14:28
@mwalenia
Copy link
Member Author

One test failure is in Task :beam-runners-google-cloud-dataflow-java-legacy-worker:test, unrelated to these changes.
Second one is checkstyle in /src/runners/direct-java/src/main/java/org/apache/beam/runners/direct/portable/job/ReferenceRunnerJobServer.java:147
, there's a Javadoc missing - also unrelated

Copy link
Contributor

@kkucharc kkucharc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job, thanks! I just added two suggestions.

@kkucharc
Copy link
Contributor

Run Java PreCommit

@mwalenia mwalenia force-pushed the BEAM-6207-add-kafka-to-synth-pubsub branch from a87a869 to b2b0b28 Compare January 25, 2019 09:15
Copy link
Contributor

@lgajowy lgajowy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Posted some suggestions (hopefully helpful). Thanks!

EDIT: forgot one thing: coudl you change a name to something abstracting from PubSub, like SyntheticDataPublisher or similar?

Copy link
Contributor

@lgajowy lgajowy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost done, some suggestions and questions emerged though. Thanks!

@mwalenia mwalenia force-pushed the BEAM-6207-add-kafka-to-synth-pubsub branch from ffaac57 to 07bc1c6 Compare January 30, 2019 12:31
Michal Walenia added 2 commits January 30, 2019 15:04
SyntheticDataPubSubPublisher now accepts --kafkaBootstrapServerAddress option
which switches it to Kafka publishing mode instead of Google PubSub mode.
Key-Value pairs are published as strings.
SyntheticDataPubSubPublisher now accepts --kafkaBootstrapServerAddress option
which switches it to Kafka publishing mode instead of Google PubSub mode.
Key-Value pairs are published as strings.
@mwalenia mwalenia force-pushed the BEAM-6207-add-kafka-to-synth-pubsub branch from 07bc1c6 to 7e1f775 Compare January 30, 2019 14:05
@lgajowy
Copy link
Contributor

lgajowy commented Jan 30, 2019

LGTM, thanks! Merging.

@lgajowy lgajowy merged commit d1ac67c into apache:master Jan 30, 2019
@mwalenia mwalenia deleted the BEAM-6207-add-kafka-to-synth-pubsub branch January 24, 2020 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants