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

Pre-Work for adding Checkpointing #3163

Merged
merged 4 commits into from
May 2, 2021

Conversation

cgardens
Copy link
Contributor

@cgardens cgardens commented May 1, 2021

What

This PR handles cleaning up some stuff that has to be done before checkpointing can happening. Doing it here to avoid adding noise to the meat of the change.

  • Destinations now are aware that they can received AirbyteMessages instead of just AirbyteRecordMessages.
  • Have the RecordWriter iface operate on lists of records (instead of streams). In practice the records were represented as lists on both sides of the interface already anyway. This interface it operating on batches that are being written to the db, so they need to be able to fit into memory anyway.

Copy link
Contributor

@michel-tricot michel-tricot left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -154,6 +154,7 @@ private static RecordWriter recordWriterFunction(final Map<String, Index> indexN
// Tools like DBT do not apply. Therefore, we need to try to write data in the most usable format
// possible that does not require alteration.
final String json = Jsons.serialize(recordStream
Copy link
Contributor

Choose a reason for hiding this comment

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

rename variable: records instead of recordStream

Copy link
Contributor

@masonwheeler masonwheeler left a comment

Choose a reason for hiding this comment

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

👍

@cgardens cgardens force-pushed the cgardens/prepare_destination_changes branch from f08ba2e to 27559b8 Compare May 1, 2021 20:22
@cgardens cgardens merged commit 4cf69ac into master May 2, 2021
@cgardens cgardens deleted the cgardens/prepare_destination_changes branch May 2, 2021 00:21
davinchia added a commit that referenced this pull request May 3, 2021
@davinchia davinchia mentioned this pull request May 3, 2021
davinchia added a commit that referenced this pull request May 3, 2021
* Fix master build.

* Revert "Pre-Work for adding Checkpointing (#3163)"

This reverts commit 4cf69ac.

* Add Mason to auto-assign list.
@cgardens cgardens mentioned this pull request May 3, 2021
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

3 participants