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

Added Kafka Source and Kafka Sink to Pulsar Connect #1557

Merged
merged 3 commits into from
Apr 13, 2018

Conversation

srkukarni
Copy link
Contributor

Motivation

Explain here the context, and why you're making that change.
What is the problem you're trying to solve.

Modifications

Describe the modifications you've done.

Result

After your change, what will change.

@srkukarni
Copy link
Contributor Author

@sijie @merlimat

Copy link
Member

@sijie sijie left a comment

Choose a reason for hiding this comment

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

looks good. one comment about the kafka version.

pom.xml Outdated
@@ -144,6 +144,7 @@ flexible messaging model and an intuitive client API.</description>
<hbc-core.version>2.2.0</hbc-core.version>
<cassandra-driver-core.version>3.4.0</cassandra-driver-core.version>
<aerospike-client.version>4.1.5</aerospike-client.version>
<kafka-client.version>0.10.0.0</kafka-client.version>
Copy link
Member

Choose a reason for hiding this comment

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

shall we just use 1.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1.0 seems a very recent release. Used 0.10 since that seems to be the most popular version out there. I'm assuming that most of the migrations will come from ppl running kafka for a while.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's already a version of kafka clients used for the wrapper. We should probably standardize on one version

@sijie sijie added this to the 2.0.0-incubating milestone Apr 12, 2018
@srkukarni
Copy link
Contributor Author

@merlimat I've standardized the versions used by both modules. Please take a look again. Thanks!

@srkukarni
Copy link
Contributor Author

retest this please

Copy link
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

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

👍

@merlimat merlimat merged commit 616e2a2 into apache:master Apr 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants