🐛 Source Convex: full_refresh stops after one page#33431
Merged
Serhii Lazebnyi (lazebnyi) merged 5 commits intoairbytehq:masterfrom Dec 18, 2023
Merged
🐛 Source Convex: full_refresh stops after one page#33431Serhii Lazebnyi (lazebnyi) merged 5 commits intoairbytehq:masterfrom
Serhii Lazebnyi (lazebnyi) merged 5 commits intoairbytehq:masterfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Octavia Squidington III (octavia-squidington-iii)
added
the
area/connectors
Contributor
Before Merging a Connector Pull RequestWow! What a great pull request you have here! 🎉 To merge this PR, ensure the following has been done/considered for each connector added or updated:
If the checklist is complete, but the CI check is failing,
|
Nipunn Koorapati (nipunn1313)
approved these changes
Dec 13, 2023
Contributor
Nipunn Koorapati (nipunn1313)
left a comment
Nipunn Koorapati (nipunn1313)
left a comment
There was a problem hiding this comment.
Nice test. Good find.
Oof. With this bugfix, initial snapshot syncs of more than one page will work.
Octavia Squidington III (octavia-squidington-iii)
added
the
area/documentation
Lee Danilek (ldanilek)
changed the title
Lee Danilek (ldanilek)
changed the title
Contributor
|
I am currently engaged in testing of this PR. |
Tim Roes (timroes)
pushed a commit
that referenced
this pull request
Dec 19, 2023
Co-authored-by: Serhii Lazebnyi <[email protected]>
Jatin Yadav (jatinyadav-cc)
pushed a commit
to ollionorg/datapipes-airbyte
that referenced
this pull request
Feb 26, 2024
Co-authored-by: Serhii Lazebnyi <[email protected]>
Jatin Yadav (jatinyadav-cc)
pushed a commit
to ollionorg/datapipes-airbyte
that referenced
this pull request
Feb 26, 2024
Co-authored-by: Serhii Lazebnyi <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Describe what the change is solving
The Convex source connector has a bug where SyncMode == full_refresh causes it to stop the sync after a single page of 128 results. This PR fixes the bug and adds tests.
It helps to add screenshots if it affects the frontend.
Before, note that only 128 documents are synced:
After, note that 1773 records are synced, which is the total number of records in the source:
How
Describe the solution
The Convex source connector does two sets of pagination, in sequence:
Currently we stop the sync when the "delta" pagination is done. But this is a problem for full-refresh sync, which doesn't do the "delta" pagination at all, which means the "delta" pagination is marked as done before the "snapshot" pagination has completed.
The solution is we return a non-None
next_page_tokenwhen //either// the "snapshot" or "delta" syncs have more data.Also add unit tests that would have caught this case.
Recommended reading order
source_convex/source.pysource-convex/unit_tests/test_streams.py🚨 User Impact 🚨
Are there any breaking changes? What is the end result perceived by the user?
The end result is that syncs that would previously have terminated early will now fully sync all data.
This only applies for users that are using full_refresh sync mode.
Therefore it is strongly recommended that users do a full sync after this change releases, to make sure their data fully syncs.
Up to reviewer whether this counts as a breaking change.
For connector PRs, use this section to explain which type of semantic versioning bump occurs as a result of the changes. Refer to our Semantic Versioning for Connectors guidelines for more information. Breaking changes to connectors must be documented by an Airbyte engineer (PR author, or reviewer for community PRs) by using the Breaking Change Release Playbook.
If there are breaking changes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.
Pre-merge Actions
Expand the relevant checklist and delete the others.
Updating a connector
Community member or Airbyter
Airbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.