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

Enhance serializers with use.latest.version config #1133

Merged
merged 4 commits into from
Jun 2, 2021

Conversation

rayokota
Copy link
Member

@rayokota rayokota commented Jun 1, 2021

Also add skip.known.types config for Protobuf serializer

@ghost
Copy link

ghost commented Jun 1, 2021

@confluentinc It looks like @rayokota just signed our Contributor License Agreement. 👍

Always at your service,

clabot

@rayokota rayokota requested a review from rnpridgeon June 1, 2021 15:18
rayokota added 2 commits June 2, 2021 09:59
Also add additional checks that both use.latest.version and
auto.register.schemas are not set
@rayokota rayokota requested a review from mhowlett June 2, 2021 17:21
.format(subject), 0)
assert register_count == 0
# Ensure latest was requested
assert test_client.counter['GET'].get('/subjects/{}/versions/latest'.format(subject)) == 1
Copy link
Contributor

Choose a reason for hiding this comment

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

there should really be more test coverage of this stuff.
but i'm ok with the effort/benefit tradeoff.

Copy link
Contributor

@mhowlett mhowlett left a comment

Choose a reason for hiding this comment

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

thanks, LGTM.

@Atheuz
Copy link

Atheuz commented Jan 8, 2022

Fixes bug where a schema lookup is performed for every ProtobufSerializer call. Made Protobuf and schema registry impractical to use for anything but the smallest producers as it would make the lookup, on my network this would take 0.2 seconds per produce. Adds up quick.

@mhowlett
Copy link
Contributor

mhowlett commented Jan 9, 2022

@Atheuz - to clarify - are you saying this PR fixed such a bug, or the bug remains in 1.8.2?

@Atheuz
Copy link

Atheuz commented Jan 10, 2022

@mhowlett this PR fixed the bug and as such is fixed in the 1.8.2 release. It just wasn't stated anywhere here, so I wanted to add a comment.

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