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.

Sign up for GitHub

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

Jump to bottom

Pull in Kafka 2.0 clients #100

Merged
merged 4 commits into from
Dec 7, 2018
Merged

Pull in Kafka 2.0 clients #100

merged 4 commits into from
Dec 7, 2018

Conversation

jonlee2
Copy link
Contributor

@jonlee2 jonlee2 commented Nov 29, 2018 •
edited
Loading

The following changes have been made in addition to bumping up the Kafka version to pull in:

  • LiKafkaConsumerImpl implements new methods in the KafkaConsumer API, including ones that introduce timeouts for existing operations like position, committed, etc.
  • org.apache.kafka.common.protocol.Security is under the package, org.apache.kafka.common.security.auth, in 2.0
  • Redundant API declarations in LiKafkaProducer and LiKafkaConsumer have been removed.

@jonlee2 jonlee2 requested a review from navina December 6, 2018 03:46
@smccauliff
Copy link
Contributor

Can we also have some basic unit tests for the new methods.

navina
navina approved these changes Dec 6, 2018
View reviewed changes
Copy link

@navina navina left a comment

Choose a reason for hiding this comment

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

Can you add more description in the PR highlighting the major changes going in? For example, the consumer API now has overrides introducing timeouts for operations like position, committed etc.

Otherwise, looks good! +1

li-apache-kafka-clients/src/main/java/com/linkedin/kafka/clients/consumer/LiKafkaConsumer.java Outdated Show resolved Hide resolved
@jonlee2
Copy link
Contributor Author

jonlee2 commented Dec 7, 2018 •
edited
Loading

Can we also have some basic unit tests for the new methods.

@smccauliff Added a test for commitSync with timeout, as discussed.

@jonlee2 jonlee2 self-assigned this Dec 7, 2018
@jonlee2 jonlee2 merged commit 1967b6b into linkedin:master Dec 7, 2018
@jonlee2 jonlee2 deleted the 2_0_migration branch December 7, 2018 22:35
@jonlee2 jonlee2 mentioned this pull request Dec 10, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smccauliff smccauliff smccauliff approved these changes

@navina navina navina approved these changes

Assignees

@jonlee2 jonlee2

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jonlee2 @smccauliff @navina