Conversation
Update poetry.lock per the recent estuary-cdk package dependency bumps. Materially for this connector, orjson was bumped from 3.11.1 to 3.11.8.
The async tests in `TestReconcileConnectorState` were being silently skipped in CI because `pytest-asyncio` was not a dev dependency. `pytest` treated async def functions as coroutines it couldn't handle and skipped them. Adding `pytest-asyncio` allows async tests to run in CI now.
…or guards `test_error_on_invalid_initial_state` and `test_error_on_none_inc_state` expected `RuntimeError` raises that were not implemented in `_reconcile_connector_state`. These tests were previously silently skipped in CI since `pytest-asyncio` was not installed, but those tests fail now that the dependency is present.
Alex-Bair
commented
Apr 7, 2026
Comment on lines
-144
to
-145
| async def test_independent_backfill_addition(self): | ||
| """Backfill is added for a store even when its inc already exists.""" |
Member
Author
There was a problem hiding this comment.
Note: the test_independent_backfill_addition test was actually testing behavior that we don't want - backfills shouldn't be added if the associated incremental state already exists; if the incremental state exists but the backfill doesn't, that means the backfill already completed! This PR actually fixed that bug already. This test wasn't updated until now since it never ran in CI without the pytest-asyncio dev dependency.
…en adding stores When a new store is added to the endpoint config of an existing capture, `_reconcile_connector_state` adds it to each resource's state. Right now, it only added stores to resource states that have both incremental _and_ backfill tasks. But that's insufficient since there are actually some resources that only have incremental tasks (bulk resources and non-bulk without `SORT_KEY` have `backfill=None`). The connector currently doesn't add stores to the state for those incremental-only resources, and that caused assertion errors in the CDK's `open_binding` function. This commit fixes that state reconciliation bug. If the store exists in the incremental state, then no reconciliation is performed. If the store doesn't exist in the incremental state, then it's added to the incremental state & it's added to the backfill state if the resource uses a distinct backfill task.
Alex-Bair
force-pushed
the
bair/source-shopify-native-fix-state-reconciliation-again
branch
from
04c6bfc to
1396ecf
Compare
Alex-Bair
deleted the
bair/source-shopify-native-fix-state-reconciliation-again
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
This PR's scope includes:
poetry.lockdue to the recent changes toestuary-cdk's dependencies in f3351b7 and 640b601.pytest-asyncioas a dev dependency so the async tests will run in CIstores into the resource state in_reconcile_resource_statewhen the resource does not have a distinct backfill task.See individual commits for more details.
Confirmed all tests now run in CI. See the snippet below for evidence that no tests were skipped: