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

Fix conversations stream bookmarking #73

Merged
merged 31 commits into from
May 8, 2024

Conversation

RushiT0122
Copy link
Contributor

Description of change

  • conversations records updated after previous sync start were missed in next sync because we were setting last sync end time as bookmark. Fixed this issue by bookmarking last sync start time.
  • Long running historical sync was stuck in loop after interruption because we were resuming sync from the beginning. Added nested filters in search_query to resume sync from the last processed conversations record.

Manual QA steps

  • Tested fixes on local environments
  • Done the PR-alpha on client connections and verified the fix

Risks

Rollback steps

  • revert this branch

@RushiT0122 RushiT0122 changed the title Tdl 25075 fix bookmarking conversations Fix conversations stream bookmarking May 1, 2024
…singer-io/tap-intercom into TDL-25075-fix-bookmarking-conversations
"sort": {
"field": self.replication_key,
"field": "id",
Copy link
Member

Choose a reason for hiding this comment

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

field is hardcoded to id, is it working for all the streams?

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 just for the Conversations stream.

@sgandhi1311
Copy link
Member

Are we writing the bookmarks for the child streams?

@RushiT0122
Copy link
Contributor Author

Are we writing the bookmarks for the child streams?

Yes (streams.py#L121)

@RushiT0122 RushiT0122 requested a review from sgandhi1311 May 6, 2024 18:11
@RushiT0122 RushiT0122 merged commit 4a9b682 into master May 8, 2024
3 checks passed
@rdeshmukh15 rdeshmukh15 mentioned this pull request May 8, 2024
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.

4 participants