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

Add MetadataClient getCommittedOffsets #1073

Merged
merged 4 commits into from
Mar 5, 2020
Merged

Add MetadataClient getCommittedOffsets #1073

merged 4 commits into from
Mar 5, 2020

Conversation

epalace
Copy link

@epalace epalace commented Mar 3, 2020

Stop using deprecated KafkaConsumer#committed method, and start using its replacement. This allows to use a set of partitions in a single call, which has better performance than iterating on the old deprecated method.

Old PR: #1071

@epalace epalace mentioned this pull request Mar 3, 2020
Closed
seglo
seglo reviewed Mar 3, 2020
View reviewed changes
Copy link
Contributor

@seglo seglo left a comment

Choose a reason for hiding this comment

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

Looks pretty good. Can you please update the docs to replace the deprecated "get committed offsets" mentions with your new API.

i.e.)

https://doc.akka.io/docs/alpakka-kafka/current/consumer-metadata.html#supported-metadata-by-metadataclient

core/src/main/scala/akka/kafka/Metadata.scala Outdated Show resolved Hide resolved
@lightbend-cla-validator
Copy link

At least one pull request committer is not linked to a user. See https://help.github.com/en/articles/why-are-my-commits-linked-to-the-wrong-user#commits-are-not-linked-to-any-user

@epalace epalace force-pushed the get_committed_offsets_in_batch branch from 2cc7447 to 547f38a Compare March 4, 2020 09:16
@lightbend-cla-validator
Copy link

At least one pull request committer is not linked to a user. See https://help.github.com/en/articles/why-are-my-commits-linked-to-the-wrong-user#commits-are-not-linked-to-any-user

@epalace epalace force-pushed the get_committed_offsets_in_batch branch from f34c728 to dc4dc63 Compare March 4, 2020 11:04
@lightbend-cla-validator
Copy link

At least one pull request committer is not linked to a user. See https://help.github.com/en/articles/why-are-my-commits-linked-to-the-wrong-user#commits-are-not-linked-to-any-user

@epalace epalace force-pushed the get_committed_offsets_in_batch branch from a4289f5 to e6a5013 Compare March 4, 2020 11:47
@epalace
Copy link
Author

epalace commented Mar 4, 2020

@seglo could you have a last look to it?

@seglo
Copy link
Contributor

seglo commented Mar 4, 2020

@seglo could you have a last look to it?

I'll take a look shortly. In the meantime do you mind squashing your commits with your GitHub user id that signed the CLA? Then the lightbend-cla-validator will stop complaining.

seglo
seglo approved these changes Mar 4, 2020
View reviewed changes
Copy link
Contributor

@seglo seglo left a comment

Choose a reason for hiding this comment

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

LGTM. We can merge once it's squashed into a commit by your CLA signed user.

ennru
ennru approved these changes Mar 5, 2020
View reviewed changes
Copy link
Member

@ennru ennru left a comment

Choose a reason for hiding this comment

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

LGTM.

@ennru ennru added this to the 2.0.3 milestone Mar 5, 2020
@ennru ennru changed the title KafkaConsumerActor: don't use deprecated KafkaConsumer#committed method Add MetadataClient getCommittedOffsets Mar 5, 2020
@ennru ennru merged commit 843db85 into akka:master Mar 5, 2020
@ennru
Copy link
Member

ennru commented Mar 5, 2020

Thank you!

@epalace epalace deleted the get_committed_offsets_in_batch branch March 6, 2020 12:05
@epalace epalace mentioned this pull request Mar 6, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seglo seglo seglo approved these changes

@ennru ennru ennru approved these changes

Assignees
No one assigned
Labels
core documentation tests
Projects
None yet
Milestone
2.0.3
Development

Successfully merging this pull request may close these issues.
4 participants
@epalace @lightbend-cla-validator @seglo @ennru