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

change kafkaConnectSource to return KeyValue type record #2902

Merged
merged 2 commits into from Nov 5, 2018

Conversation

jiazhai
Copy link
Member

@jiazhai jiazhai commented Nov 1, 2018

change kafkaConnectSource to return KeyValue type record.

@jiazhai jiazhai requested a review from sijie November 1, 2018 15:59
public byte[] getValue() {
return valueConverter.fromConnectData(
public KeyValue<byte[], byte[]> getValue() {
byte[] keyBytes = keyConverter.fromConnectData(
Copy link
Member

Choose a reason for hiding this comment

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

line 157 would also convert the original key. I think we should try to cache keyBytes, so either getKey or getValue is called, the key is converted to keyBytes, any subsequent calls, will just use the cached bytes.

public KeyValue<byte[], byte[]> getValue() {
byte[] keyBytes = keyConverter.fromConnectData(
srcRecord.topic(), srcRecord.keySchema(), srcRecord.key());
byte[] valueBytes = valueConverter.fromConnectData(
Copy link
Member

Choose a reason for hiding this comment

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

same here to cache value bytes.

String line1 = "This is the first line\n";
os.write(line1.getBytes());
os.flush();
log.info("write 2 lines.");
kafkaConnectSource.getOffsetWriter().offset(
Copy link
Member

Choose a reason for hiding this comment

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

what is the reason of changing these lines here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @sijie , we already do the offset handling in KafkaConnecSource.

@sijie sijie added this to the 2.3.0 milestone Nov 5, 2018
@sijie sijie merged commit 5de1ef7 into apache:master Nov 5, 2018
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.

None yet

2 participants