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

feat(microservices): Add options param to serializer #9200

Merged
merged 1 commit into from
Mar 1, 2022

Conversation

guiruiz
Copy link
Contributor

@guiruiz guiruiz commented Feb 15, 2022

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior?

Currently the Kafka Client/Server calls the serializer passing only the packet/message value as parameter.

Issue Number: N/A

What is the new behavior?

From now the Kafka Client is calling the serializer passing the packet value and also an options object containing the pattern (topic name) as parameters.
With the optional options parameter the Serializer interface is now matching the Deserializer interface.

Does this PR introduce a breaking change?

  • Yes
  • No

This change doesn't introduce a breaking change because the options parameter is optional.

Other information

I'm deserializing/serializing messages using Avro Schemas with the Schema Registry, in order to do that the custom serializer needs to be aware of the topic/pattern name of each message.
Since this is an issue that other people may have I decided to contribute opening this PR.

@coveralls
Copy link

Pull Request Test Coverage Report for Build e5e1fa2b-d372-493f-9841-71d4ea30cf94

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 94.156%

Totals Coverage Status
Change from base Build d711baf6-cae3-43ad-bc15-20e80f939ea3: 0.0%
Covered Lines: 5720
Relevant Lines: 6075

💛 - Coveralls

@kamilmysliwiec
Copy link
Member

LGTM

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