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

fix(source-intercom): incremental not emitting state #52132

Merged
merged 8 commits into from
Jan 29, 2025

Conversation

darynaishchenko
Copy link
Collaborator

@darynaishchenko darynaishchenko commented Jan 24, 2025

What

https://github.com/airbytehq/oncall/issues/7302
cdk doesn’t emit state while reading records, because checkpoint_interval for declarative stream is None. And in the end of the read cdk also doesn’t emit state because new_state == previous_state.

How

fixed get_stream_state to return a copy of current state not ref to the state. updated to latest cdk with fix for checkpointing

Review guide

User Impact

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

@darynaishchenko darynaishchenko self-assigned this Jan 24, 2025
Copy link

vercel bot commented Jan 24, 2025

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

Name Status Preview Comments Updated (UTC)
airbyte-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 29, 2025 0:51am

@octavia-squidington-iii octavia-squidington-iii added area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/intercom labels Jan 24, 2025
@maxi297
Copy link
Contributor

maxi297 commented Jan 24, 2025

@lazebnyi can you take this one?

@maxi297 maxi297 requested a review from lazebnyi January 24, 2025 14:51
@darynaishchenko
Copy link
Collaborator Author

Regression test results:
run regression test locally, because using it via gha lead to error for connection version as we pinned v0.8.3

State count 0.9.0 vs dev:
39 64 diff=25
dev version has more states as we fixed custom cursor and updated cdk to latest version with a fix for checkpointing.

test_catalog_are_the_same[failed]

full refresh streams are now not resumable, related on cdk update. Acceptable and expected.

test_all_records_are_the_same_with_state[failed for when control 0.8.3, but passed when control is 0.9.0]

Expected: on latest cdk version records don't have some fields when their value Null

@darynaishchenko
Copy link
Collaborator Author

darynaishchenko commented Jan 29, 2025

/approve-regression-tests

Check job output.

✅ Approving regression tests

@darynaishchenko darynaishchenko merged commit 2359e36 into master Jan 29, 2025
30 checks passed
@darynaishchenko darynaishchenko deleted the daryna/source-intercom/fix-incremental branch January 29, 2025 13:48
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/intercom
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants