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: Migrate to low code #36823

Merged
merged 16 commits into from
Apr 29, 2024

Conversation

tolik0
Copy link
Contributor

@tolik0 tolik0 commented Apr 4, 2024

What

Migrate to low code. Add new stream Ticket Activities: https://github.com/airbytehq/product-request-backlog/issues/72
This PR adds many changes to different streams. For simplicity, all errors from the previous version are described in the file: https://docs.google.com/document/d/1JeC4fPl9zqgPJZLJs7KVRnCk--v8cO8hKyJHAeKoXVw/edit

Here is a short description of the changes:

  • Articles - filtering with start_time works, but sorting doesn't work ( no error, but results are not sorted) - preserved sorting params as they are described in the API docs. - returned back to Python implementation.
  • Audit Logs - start_time is not applicable, no sorting - Changed to filter[created_at][] and sort parameters described in the API docs.
  • Group Memberships - start_time is not applicable, sorting doesn't work - Changed to semi incremental with local filtering.
  • Groups - start_time is not applicable - Changed to semi incremental with local filtering.
  • Macros - start_time is not applicable - Changed to semi incremental with local filtering.
  • Organization Fields - start_time is not applicable, no sorting - Changed to semi incremental with local filtering.
  • Organization Memberships - start_time is not applicable, no sorting - Changed to semi incremental with local filtering.
  • Posts - start_time is not applicable - Changed to semi incremental with local filtering, add sorting. - returned back to Python implementation
  • Satisfaction Ratings - sorting is not working - Changed sorting to sort=created_at, updated_at is not supported.
  • Ticket Audits - sorting is not working, start_time was not used but it is not applicable - Data is sorted in descending order by default, but preserving sorting parameters as data feed is used to stop iterating when records are earlier than start date or state.
  • Ticket Fields - start_time is not applicable - Changed to semi incremental with local filtering
  • Ticket Forms - start_time is not applicable - Changed to semi incremental with local filtering
  • Ticket Metrics - start_time is not applicable - Data is sorted by created at, so semi-incremental with local filtering is used - reverted to Pyton code
  • Ticket Skips - start_time is not applicable - Changed to semi incrementla with local filtering, add sorting. Didn't add data feed as not sure if sorted by created or updated.
  • Topic - nor start_time (no error but results are not filtered), nor sorting is not working - Changed to semi-incremental

Substreams are in Python as currently, low code doesn't support reading parent streams with a state.

🚨 User Impact 🚨

No breaking changes. A new stream added: Ticket Activities.

Copy link

vercel bot commented Apr 4, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
airbyte-docs ⬜️ Ignored (Inspect) Visit Preview Apr 29, 2024 0:25am

@tolik0 tolik0 force-pushed the tolik0/source-zendesk-support/migrate-to-low-code branch from 9cc4003 to 8406ada Compare April 5, 2024 12:15
@octavia-squidington-iii octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Apr 11, 2024
@tolik0 tolik0 self-assigned this Apr 11, 2024
@tolik0 tolik0 marked this pull request as ready for review April 12, 2024 15:42
@tolik0 tolik0 requested a review from a team April 12, 2024 15:42
@octavia-squidington-iv octavia-squidington-iv requested a review from a team April 12, 2024 15:43
@tolik0 tolik0 force-pushed the tolik0/source-zendesk-support/migrate-to-low-code branch from 0218f9f to 2854c52 Compare April 15, 2024 09:19
@tolik0
Copy link
Contributor Author

tolik0 commented Apr 17, 2024

Regression Tool results:

catalog:
A new stream was added - Ticket Activities.

test_record_schema_match_with_state:
brands, topics - normalization
ticket_fields - records were filtered by time

Read:

  • custom_roles - control version doesnt filter records by time
  • ticket_fields - control version doesnt filter records by time
  • organization_memberships - control version doesnt filter records by time
  • group_memberships - control version doesnt filter records by time
  • ticket_metric_events - difference in records are either due to cursor comparison changed from > to >=, or records that were extracted after the state of the first read.
  • satisfaction_rating - +1 record - cursor comparison changed from > to >=
  • ticket_audits - new records were added
  • groups - +1 record - cursor comparison changed from > to >=
  • ticket_forms - +1 record - cursor comparison changed from > to >=
  • ticket_fields - +1 record - cursor comparison changed from > to >=
  • organizations - control version doesn't use start date
  • groups - control version doesnt filter records by time

@octavia-squidington-iv octavia-squidington-iv requested a review from a team April 17, 2024 16:12
@octavia-squidington-iv octavia-squidington-iv requested a review from a team April 18, 2024 13:29
@tolik0 tolik0 force-pushed the tolik0/source-zendesk-support/migrate-to-low-code branch from ffb11e3 to 303c6d3 Compare April 22, 2024 17:51
Copy link
Collaborator

@lazebnyi lazebnyi left a comment

Choose a reason for hiding this comment

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

lgtm, just need fix CAT

@tolik0 tolik0 force-pushed the tolik0/source-zendesk-support/migrate-to-low-code branch from 3634e60 to 213758d Compare April 25, 2024 12:28
@bnchrch
Copy link
Contributor

bnchrch commented Apr 25, 2024

Hey just running the regression test for the first time.

A few things stood out to me I wondered if you could explain

1. We saw significantly more record messages (+262) specifically for ticket_audits.

image

Is that expected? If so why?

2. Type changes

SLA policy had a type change int -> string
image
Is there any chance of a down stream implication because of the change from int to string?

source_id and actor_id from int -> float
image
Is there any chance of a down stream implication because of the change int to float?


Im sure these are obvious answers, just want to sharpen my understanding.

@tolik0
Copy link
Contributor Author

tolik0 commented Apr 25, 2024

Hi, @bnchrch,

  1. Great catch! You identified an issue where records in the macros and ticket_audits streams weren't being filtered by time. I've already pushed a fix for that. For other streams, the behaviour is expected: they will either include new records or records with timestamps matching the cursor.
  2. Additionally, the data types have been corrected in the normalization process to align with the defined schemas. Here is the correct type of source_id field: .

Copy link
Contributor

@bnchrch bnchrch left a comment

Choose a reason for hiding this comment

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

TLDR: LGTM

Thanks for the reply @tolik0 !

I reran the tool and it looks like were nearly identical with the exception of a very minor +7 records for a stream

image

But I think thats implementation details and nothing to be worried about.

Approved :)

@tolik0 tolik0 force-pushed the tolik0/source-zendesk-support/migrate-to-low-code branch from 86e183c to f2b16be Compare April 29, 2024 12:25
@artem1205 artem1205 merged commit 918aa31 into master Apr 29, 2024
37 checks passed
@artem1205 artem1205 deleted the tolik0/source-zendesk-support/migrate-to-low-code branch April 29, 2024 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/zendesk-support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants