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

[dagster-fivetran] Implement sync_and_poll method in FivetranClient #26061

Merged

Conversation

maximearmstrong
Copy link
Contributor

@maximearmstrong maximearmstrong commented Nov 20, 2024

Summary & Motivation

This PR uses the sync and poll methods implemented in previous PRs to implement sync_and_poll in FivetranClient. This method will be used in a subsequent PR to materialize Fivetran assets.

Tests are added to test the full sync and poll behavior.

How I Tested These Changes

Additional unit tests with BK

Copy link
Contributor Author

maximearmstrong commented Nov 20, 2024

@maximearmstrong maximearmstrong force-pushed the maxime/implement-base-poll-method-fivetran-client branch from 469b919 to 6fff27d Compare November 20, 2024 23:25
@maximearmstrong maximearmstrong force-pushed the maxime/implement-sync-and-poll-method-fivetran-client branch from 0df7a52 to 4548acb Compare November 20, 2024 23:25
@maximearmstrong maximearmstrong force-pushed the maxime/implement-base-poll-method-fivetran-client branch from 6fff27d to 77163b2 Compare November 21, 2024 18:06
@maximearmstrong maximearmstrong force-pushed the maxime/implement-sync-and-poll-method-fivetran-client branch from 4548acb to 1368cac Compare November 21, 2024 18:06
@maximearmstrong maximearmstrong force-pushed the maxime/implement-base-poll-method-fivetran-client branch from 77163b2 to a55e620 Compare November 21, 2024 21:08
@maximearmstrong maximearmstrong force-pushed the maxime/implement-sync-and-poll-method-fivetran-client branch from 1368cac to 8610c37 Compare November 21, 2024 21:08
@maximearmstrong maximearmstrong force-pushed the maxime/implement-base-poll-method-fivetran-client branch from a55e620 to 7ff0145 Compare November 21, 2024 23:45
@maximearmstrong maximearmstrong force-pushed the maxime/implement-sync-and-poll-method-fivetran-client branch 2 times, most recently from 7f056cc to 946d522 Compare November 21, 2024 23:48
@maximearmstrong maximearmstrong self-assigned this Nov 21, 2024
@maximearmstrong maximearmstrong marked this pull request as ready for review November 21, 2024 23:50
test_failed_at = TEST_PREVIOUS_MAX_TIME_STR
# Set `failed_at` as more recent that `succeeded_at` if the sync and poll process is expected to fail
if not succeed_at_end:
test_succeeded_at, test_failed_at = test_failed_at, test_succeeded_at
Copy link
Contributor

Choose a reason for hiding this comment

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

this is kind of a weird way to do this but I think it's fine. Just add a comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it in 05442c2 to make it clearer

Copy link
Contributor

Nice, thanks

@maximearmstrong maximearmstrong force-pushed the maxime/implement-base-poll-method-fivetran-client branch from 0d4c5c3 to 919e38e Compare November 25, 2024 13:06
@maximearmstrong maximearmstrong force-pushed the maxime/implement-sync-and-poll-method-fivetran-client branch from 27b0a0f to e023cd6 Compare November 25, 2024 13:06
@maximearmstrong maximearmstrong force-pushed the maxime/implement-base-poll-method-fivetran-client branch from 919e38e to b85bbe8 Compare November 25, 2024 16:50
@maximearmstrong maximearmstrong force-pushed the maxime/implement-sync-and-poll-method-fivetran-client branch from e023cd6 to 533e89a Compare November 25, 2024 16:50
@maximearmstrong maximearmstrong force-pushed the maxime/implement-base-poll-method-fivetran-client branch from b85bbe8 to c7250c1 Compare November 25, 2024 23:22
@maximearmstrong maximearmstrong force-pushed the maxime/implement-sync-and-poll-method-fivetran-client branch from 533e89a to 936186f Compare November 25, 2024 23:22
@maximearmstrong maximearmstrong force-pushed the maxime/implement-base-poll-method-fivetran-client branch from c7250c1 to 9161976 Compare November 26, 2024 22:50
@maximearmstrong maximearmstrong force-pushed the maxime/implement-sync-and-poll-method-fivetran-client branch from 936186f to 5637798 Compare November 26, 2024 22:50
@maximearmstrong maximearmstrong force-pushed the maxime/implement-base-poll-method-fivetran-client branch from 9161976 to 13b8e6d Compare November 26, 2024 23:21
@maximearmstrong maximearmstrong force-pushed the maxime/implement-sync-and-poll-method-fivetran-client branch from 5637798 to 90e1e6e Compare November 26, 2024 23:21
@maximearmstrong maximearmstrong force-pushed the maxime/implement-base-poll-method-fivetran-client branch from 13b8e6d to f039336 Compare November 27, 2024 13:32
@maximearmstrong maximearmstrong force-pushed the maxime/implement-sync-and-poll-method-fivetran-client branch from 90e1e6e to 3d7d017 Compare November 27, 2024 13:32
@maximearmstrong maximearmstrong force-pushed the maxime/implement-base-poll-method-fivetran-client branch from f039336 to 4b0d41a Compare November 27, 2024 13:44
@maximearmstrong maximearmstrong force-pushed the maxime/implement-sync-and-poll-method-fivetran-client branch from 3d7d017 to 45db9a9 Compare November 27, 2024 13:44
Base automatically changed from maxime/implement-base-poll-method-fivetran-client to master November 27, 2024 14:01
@maximearmstrong maximearmstrong force-pushed the maxime/implement-sync-and-poll-method-fivetran-client branch from 45db9a9 to 924009c Compare November 27, 2024 14:05
@maximearmstrong maximearmstrong merged commit d1742d2 into master Nov 27, 2024
1 check passed
@maximearmstrong maximearmstrong deleted the maxime/implement-sync-and-poll-method-fivetran-client branch November 27, 2024 14:24
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.

2 participants