source-zendesk-support: update ticket_audits to checkpoint during backfills
#2288
Conversation
This file contains 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
…ackfills The previously used `/ticket_audits` endpoint used to list all ticket audits always returns results in descending order and did not let us specify a date window/start date to filter what results are returned. This prevented the connector from checkpointing during backfills, and if backfills spanned 24+ hours, they wouldn't complete since the CDK restarts the connector after it's been running for 24 hours. The `ticket_audits` stream now uses `tickets` as a parent stream, fetching an updated ticket then fetching the audits for that ticket.
When testing out changes to the `ticket_audits` stream, I encountered schema violations. These updates are my attempt to address the schema violations, but I only tested over a short period of time, so further schema updates may be necessary later too.
…ings This commit applies the same transformation logic to `tickets` like in 1b8df5d.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description:
The previously used
/ticket_auditsendpoint used to list all ticket audits always returns results in descending order and did not let us specify a date window/start date to filter what results are returned. This prevented the connector from checkpointing during backfills, and if backfills spanned 24+ hours, they wouldn't complete since the CDK restarts the connector after it's been running for 24 hours.The
ticket_auditsstream now usesticketsas a parent stream, fetching an updated ticket then fetching the audits for that ticket. Although this makes the stream slower since it now makes more HTTP requests to get results, this is the only way I found to getticket_auditsin ascending order & let us checkpoint progress for backfills spanning 24+ hours.Additional scope in this PR includes:
schemas/ticket_audits.jsonto account for schema violations encountered during testingticketsto strings. This is the same processing implemented in source-zendesk-support: handle bools inticket_skipsdocuments #1802. This was also motivated by schema violations encountered during testing.Snapshot changes are expected. Discover snapshot changes are due to the changes to
ticket_auditsand schema widening. Capture snapshot changes are due to fields Zendesk added to some streams a while ago. These fields have been present and stable for a couple months now, so it should be alright to add them to the capture snapshot.Workflow steps:
(How does one use this feature, and how has it changed)
Documentation links affected:
(list any documentation links that you created, or existing ones that you've identified as needing updates, along with a brief description)
Notes for reviewers:
Tested on a local stack. Confirmed:
ticket_auditscheckpoints during backfills.ticketsandusersstreams' behaviors have not changed.All existing captures should backfill their
ticket_auditsstream after this PR is merged due to the state change.This change is