-
Notifications
You must be signed in to change notification settings - Fork 4.6k
✨ Source Zendesk Support: user identities to low-code #55710
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
base: issue-11894/posts-child-streams-migration-to-low-code
Are you sure you want to change the base?
✨ Source Zendesk Support: user identities to low-code #55710
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
||
output = read_stream("user_identities", SyncMode.full_refresh, config) | ||
|
||
assert output.most_recent_state.stream_state.__dict__ == {"updated_at": str(most_recent_cursor_value.int_timestamp)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only change between the previous implementation and the new one i.e. that state goes from ISO8601 to epoch as a string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm curious about one part of the user identities stream, but not a huge issue once you answer it
$ref: "#/definitions/after_url_paginator" | ||
pagination_strategy: | ||
$ref: "#/definitions/after_url_paginator/pagination_strategy" | ||
stop_condition: "{{ config.get('ignore_pagination', False) or response.get('end_of_stream') }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps a dumb question, but why the change from existing stop condition being used by the users
stream that used stop_condition: '{{ response.get("end_of_stream") }}'
based on what I saw on the python UserIdentities
implementation you deleted, I didn't see where we needed to leverage the ignore_pagination config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good question and here would be a change in behavior. This is something I've been struggling with and I decided to status quo it (i.e. try to keep the same behavior as much as possible) but it does work here.
Context:
The Python code has a config switch called ignore_pagination
which ignores the pagination (for real! you can check here). Surprisingly, this is used in prod:
prod-configapi=> select distinct(workspace_id) from actor where actor_definition_id = '79c1aa37-dae3-42ae-b333-d1c105477715' and configuration->>'ignore_pagination' = 'true';
workspace_id
--------------------------------------
112412bb-78aa-4ca6-aace-ae0b83be2215
2d5722fc-2d17-4734-830a-a2177afe069b
57433aae-b70d-464c-b914-6a2145e62cab
5d0fba8a-f664-4ea2-8c74-92584f3aadc4
8165259f-d4a9-4c05-a5ac-c0cf6a77a209
cee897d1-519e-41fb-a093-47c0d128ff26
112412bb-78aa-4ca6-aace-ae0b83be2215 is internal and all the other are for the same organization so in theory, we only have one usage. Based on this PR, it [introduced ignore_pagination config variable to speed up SATs]. Hence it seems like users in prod shouldn't use it. Adding to that, the config is hidden and has been hidden since it's creation so I assume they created this using the API. I would be on board with removing this. I'll try to figure out how we handle this single user...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've reached out to support that have reached out to the client. Support told me we could go forward assuming this would be removed. I'll do the clean up on all the branches
…into issue-11894/user-identities-to-low-code
What
Partially addresses https://github.com/airbytehq/airbyte-internal-issues/issues/11894
Migrating user_identities to low-code.
How
Adding tests, implementing stream in the manifest.yaml and removing the code in the python code.
Review guide
User Impact
This change is mostly to ease maintenance for devs and there shouldn't be any change in behavior.
Can this PR be safely reverted and rolled back?
As mentioned here the state will change and is not backward compatible. If we are worried about that, we can do a forward compatible change in the current version.