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

source-zendesk-support-native: add streams to achieve parity with imported connector #2407

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

Alex-Bair
Copy link
Member

@Alex-Bair Alex-Bair commented Feb 14, 2025

Description:

This PR's scope includes:

  • Adding streams to bring source-zendesk-support-native on (and above) stream parity with the imported source-zendesk-support.
    • Added streams include:
      • account_attributes
      • automations
      • custom_roles
      • group_memberships
      • organizations
      • post_comment_votes
      • post_comments
      • post_votes
      • posts
      • schedules
      • sla_policies
      • ticket_forms
      • topics
  • Increasing the default interval for resources that have infrequent updates.
  • Conditionally discover account_attributes and audit_logs if the provided credentials have access to the appropriate endpoints.

See individual commits for more details.

Workflow steps:

(How does one use this feature, and how has it changed)

Documentation links affected:

Documentation should be updated to reflect the new streams & mark the imported source-zendesk-support as deprecated.

Notes for reviewers:

Tested on a local stack. Confirmed all streams capture data & complete.


This change is Reviewable

…ntermittent snapshot test failures

When testing locally, I'd intermittently get snapshot test failures
because a start_time param was too recent. It turns out that Zendesk
returns that error when the start_time is 60 or fewer seconds from the
present. Bumping the delay up to 61 seconds avoids these errors.
The `organizations` stream uses the incremental time based export
endpoint, which has the following quirks:
- A 10 requests / minute rate limit
- The next page is fetched using the updated_at timestamp of the last
  result in the current response. This means the next page will always
  contain at least one duplicate record (the last record of the previous
  page).

Because of these quirks:
- A super basic rate limiting strategy leveraging
  asyncio.Lock is used to only make 1 request every 6 seconds to
  incremental time export endpoints.
- An error is raised if we detect 1,000+ organizations updated at the
  same time. If we didn't the connector would be stuck looping & making
  the same request endlessly since the max page size is 1,000.
- Checkpoints can't be emitted at page breaks since the next page's
  first result overlaps with the previous page's end, and it's possible
  to miss records between connector restarts with that strategy.
Some Zendesk accounts don't have access to `audit_logs` or
`account_attributes`, so we don't discover them if receive a specific
403 error trying to hit the associated endpoint.

There may be other streams that should be conditionally discovered, but
they can be handled later when discovering them becomes an issue.
@Alex-Bair Alex-Bair marked this pull request as ready for review February 14, 2025 22:50
@Alex-Bair Alex-Bair changed the title source-zendesk-support-native: add streams source-zendesk-support-native: add streams to achieve parity with imported connector Feb 14, 2025
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.

1 participant