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

Feature/use reader schema for specific records #140

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

AFrieze
Copy link
Collaborator

@AFrieze AFrieze commented Nov 20, 2017

The KafkaAvroDeserializer uses the writer schema retrieved from the incoming message to deserialize messages into GenericRecords. This leads to default values in a newer Reader schema being ignored. This PR adds a new deserializer that is configured with the appropriate reader schema based off a source. It is worth noting that this PR removes the getQueue(String name) and getTopic(String name) from the ForkliftConnector interface.

* @throws ConnectorException if an error occurred interacting with the connector
* @throws RuntimeException if reading from a queue is not supported
*/
ForkliftConsumerI getQueue(String name) throws ConnectorException;
Copy link
Owner

Choose a reason for hiding this comment

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

Might be good to stub these in to call getConsumerForSource to avoid backward compatibility issues.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How much of a concern is this given that these methods are mostly internal? My rationale for removing these is that more and more functionality is moving into the "Source" concept and if keep these methods around we could have diverging logic in places.

@dcshock
Copy link
Owner

dcshock commented Jul 17, 2020

It looks like we forced this into develop. @Kuroshii @AFrieze can this PR be closed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants