-
Notifications
You must be signed in to change notification settings - Fork 4.6k
✨ Source Zendesk Support: migrate posts to low-code #55680
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-add-tests-and-prepare-for-migration
Are you sure you want to change the base?
✨ Source Zendesk Support: migrate posts to low-code #55680
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/manifest.yaml
Show resolved
Hide resolved
.with_record(PostsRecordBuilder.posts_record().with_cursor(datetime_to_string(_START_DATE.add(days=1)))) | ||
.build(), | ||
) | ||
|
||
output = read_stream("posts", SyncMode.full_refresh, config) | ||
|
||
assert output.most_recent_state.stream_state == AirbyteStateBlob({"updated_at": most_recent_cursor_value}) | ||
assert output.most_recent_state.stream_state == AirbyteStateBlob({"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 should be the only change in terms of behavior
WIP:
|
stop_condition: "{{ config.get('ignore_pagination', False) or not response['meta']['has_more'] }}" | ||
record_selector: | ||
$ref: "#/definitions/retriever/record_selector" | ||
schema_normalization: None |
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 required because schema is a bit loose: for example, instead of integer as id
, it is number
which cast fields as float
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.
small suggestions, but nothing to block on
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/manifest.yaml
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/manifest.yaml
Show resolved
Hide resolved
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.
LGTM!
…into issue-11894/posts-migration-to-low-code
What
Partially addresses https://github.com/airbytehq/airbyte-internal-issues/issues/11894
Following the addition of tests in #55676, we can now migration the posts stream to low-code. We still keep the Python implementation because it is used as a parent for other streams. Those streams will require a breaking change as they are not per partition although they have a parent stream.
How
Review guide
User Impact
None, the change is for devs to have easier maintenance
Can this PR be safely reverted and rolled back?
Note: This change can be reverted to #55676 as we have a made a forward compatible version as shown by test
test_given_input_state_with_low_code_format_when_read_then_set_state_value_to_most_recent_cursor_value
from this PR