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
🐛 Source Google Ads: Fix incremental events streams #32102
🐛 Source Google Ads: Fix incremental events streams #32102
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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,
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please describe what the state used to be and what it will be?
What did slices look like and what will they look like?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, given changes like these, I'd ask you to cover them with unit tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unit Test is required for the new method added. LGTM, otherwise.
ea19d76
to
3ffa6ac
Compare
@davydov-d The state structure itself has stayed the same. The issue with the previous version was that the state after the first sync wasn't in the expected format; specifically, it was missing the customer ID. Here's the correct version of the state:
Regarding the slices, their structure remains the same. The only modification I made was to split slice parameter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
What
This PR resolves issues with incremental streams that utilize the Change Events resource.
Closes: https://github.com/airbytehq/oncall/issues/3257
How
query_error: FILTER_HAS_TOO_MANY_VALUES
error.