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

Sp 1085 #1093

Merged
merged 6 commits into from
Jan 16, 2023
Merged

Sp 1085 #1093

merged 6 commits into from
Jan 16, 2023

Conversation

tenthe
Copy link
Contributor

@tenthe tenthe commented Jan 14, 2023

Purpose

Fixes #1085.

Previously, the FileStreamProtocol required another sink (SendToBrokerReplayAdapterSink) for the adapter preprocessing pipeline.
The reason for this was that the FileStreamProtocol needs to adjust the timestamp for correct replay, but it was the only adapter / protocol that used a different sink.
The SendToBrokerReplayAdapterSink class was deleted and the code was moved directly to the FileStreamProtocol.
Further, some minor refactoring was performed.

This should not be the final rewrite of the protocol, it is just a clean up step to better refactor the connect api.

Remarks

PR introduces (a) breaking change(s): no

PR introduces (a) deprecation(s): no


@Override
public EventSchema getResultingEventSchema() {
return null;
Copy link
Member

Choose a reason for hiding this comment

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

Why does this method always return null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is only temporary. In the future we probably will replace the class AdapterEventPreviewPipeline with a regular AdapterPipeline that uses a different sink.
I had to add it here to remove the dependency of the FileStreamProtocol with the internal adapter implementation.
If you have an idea on how to remove it, I'm happy to discuss it. If there is no quick solution, I would suggest to leave it as it is and remove it during the revision of the connect API.

@tenthe tenthe merged commit f84a131 into dev Jan 16, 2023
@tenthe tenthe deleted the SP-1085 branch January 16, 2023 17:28
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.

Refactor FileStreamProtocol
2 participants