source-zendesk-support: switch Groups stream to use cursor pagination #2012
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
Previously, the `Groups` stream was inheriting from the `BaseSourceZendeskSupportStream` class, meaning it paginated by retrieving the total number of results from a `/count` endpoint, calculated how many pages there should be based on that count, then concurrently sent requests for those pages of results. When I added the `exclude_deleted` param earlier to all `Groups` requests, I didn't ensure that the `/count` endpoint would include deleted groups. It does not, and it doesn't accept the `exclude_deleted` query param. So when the connector was calculating how many pages there should be, it wasn't including deleted groups in the total count, and we were missing results. Instead of keeping `Groups` as a `BaseSourceZendeskSupportStream` that uses this older method of pagination, I switched `Groups` to use cursor pagination. The stream's behaviour and document schema remained the same, only how it queries the Zendesk API changed. We should move `Macros` and `TicketFields` off of `BaseSourceZendeskSupportStream` too, but that can be done at a later time.
Alex-Bair
added
the
change:unplanned
Zendesk Support's API is no longer returning the `configuration.explore_reports` field for custom roles. This reverts the snapshot change done in PR #1991.
|
In the 30 minutes from when I tested locally & created the PR, Zendesk Support's API stopped including the |
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:
Previously, the
Groupsstream was inheriting from theBaseSourceZendeskSupportStreamclass, meaning it paginated by retrieving the total number of results from a/countendpoint, calculated how many pages there should be based off that count, then concurrently sent requests for those pages of results. When I added theexclude_deletedparam earlier to allGroupsrequests, I didn't ensure that the/countendpoint would include deleted groups. It does not, and it doesn't accept theexclude_deletedquery param. So when the connector was calculating how many pages there should be, it wasn't including deleted groups in the total count, and we were missing results.Instead of keeping
Groupsas aBaseSourceZendeskSupportStreamthat uses this older method of pagination, I switchedGroupsto use cursor pagination. The stream's behaviour and document schema remained the same, only how it queries the Zendesk API changed.We should move
MacrosandTicketFieldsoff ofBaseSourceZendeskSupportStreamtoo since their current pagination method has limitations that don't exist for the newer cursor pagination-based classes, but that can be done at a later time.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:
Groupswere retrieved.All
Groupsbindings for existing tasks will need backfilled after merging this PR to clear out previous state.This change is