Skip to content

Feature/use reader schema for specific records#140

Open
AFrieze wants to merge 2 commits into
developfrom
feature/use-reader-schema-for-specific-records
Open

Feature/use reader schema for specific records#140
AFrieze wants to merge 2 commits into
developfrom
feature/use-reader-schema-for-specific-records

Conversation

@AFrieze
Copy link
Copy Markdown
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.

@AFrieze AFrieze requested review from Kuroshii and dcshock November 20, 2017 23:06
* @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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

2 participants