source-stripe-native: Support Stripe Connected Accounts#2467
Merged
Conversation
JustinASmith
force-pushed
the
jsmith/stripe-native-connected-accounts
branch
7 times, most recently
from
588a43d to
eb9ac3b
Compare
williamhbaker
left a comment
williamhbaker
left a comment
Member
There was a problem hiding this comment.
Looking really good; made a few comments for consideration, nothing major.
Alex-Bair
reviewed
Mar 5, 2025
JustinASmith
force-pushed
the
jsmith/stripe-native-connected-accounts
branch
from
eb9ac3b to
8213252
Compare
Enhance the capture common module to support running multiple subtasks for incremental and backfill captures with independent states. This allows more flexible capture configurations where different subtasks can track their own progress and state within a single binding.
JustinASmith
force-pushed
the
jsmith/stripe-native-connected-accounts
branch
from
8213252 to
c943ff8
Compare
JustinASmith
commented
Mar 7, 2025
Comment on lines
+51
to
+79
| if ( | ||
| not validate.lastCapture | ||
| or ( | ||
| validate.lastCapture.config.config.get("capture_connected_accounts") | ||
| == validate.config.capture_connected_accounts | ||
| ) | ||
| or not validate.lastCapture.bindings | ||
| ): | ||
| resources = await all_resources(log, self, validate.config) | ||
| resolved = common.resolve_bindings(validate.bindings, resources) | ||
| return common.validated(resolved) | ||
|
|
||
| prev_bindings_by_name = { | ||
| binding.resourceConfig.name: binding.backfill | ||
| for binding in validate.lastCapture.bindings | ||
| } | ||
|
|
||
| for current_binding in validate.bindings: | ||
| resource_name = current_binding.resourceConfig.name | ||
| if ( | ||
| resource_name in prev_bindings_by_name | ||
| and current_binding.backfill <= prev_bindings_by_name[resource_name] | ||
| ): | ||
| raise ValidationError( | ||
| [ | ||
| "Cannot change the `capture_connected_accounts` property without backfilling all collections again." | ||
| ] | ||
| ) | ||
|
|
Contributor
Author
There was a problem hiding this comment.
Validation logic to require a backfill of all collections when the capture_connected_accounts endpoint config property changes
JustinASmith
commented
Mar 7, 2025
Comment on lines
+40
to
+52
| class ValidateLastCaptureEndpointConfig(BaseModel): | ||
| config: dict[str, Any] | ||
|
|
||
|
|
||
| class ValidateLastCapture(ValidateBase, Generic[EndpointConfig, ResourceConfig]): | ||
| config: ValidateLastCaptureEndpointConfig | ||
|
|
||
|
|
||
| class Validate(ValidateBase, Generic[EndpointConfig, ResourceConfig]): | ||
| config: EndpointConfig | ||
| lastCapture: ValidateLastCapture[EndpointConfig, ResourceConfig] | None = None | ||
|
|
||
|
|
Contributor
Author
There was a problem hiding this comment.
Updated Validate models to include lastCapture spec. Currently uses a work around due to issues serializing lastCapture into the EndpointConfig model.
williamhbaker
approved these changes
Mar 7, 2025
williamhbaker
left a comment
williamhbaker
left a comment
Member
There was a problem hiding this comment.
Updates LGTM! I think this is fine as-is, with the workaround for the lastCapture issue. That's probably worth looking into more, but for now this seems good.
Enhance the Stripe source connector to support capture across multiple connected accounts. Key changes include: - Add account_id field to Stripe object models - Fetch connected account and platform account IDs during resource discovery - Modify incremental and backfill fetch methods to support account-specific replication using `Stripe-Account` HTTP header - Update resource generation to create subtasks for each connected account each with their own state - Added validation to require backfilling all collections when `capture_connected_accounts` property changes
JustinASmith
force-pushed
the
jsmith/stripe-native-connected-accounts
branch
from
c943ff8 to
ceb9c3b
Compare
Alex-Bair
added a commit
that referenced
this pull request
Jun 10, 2026
When introducing the capability to have multiple subtasks for a single binding in #2467, we refrained from adding support for multiple `fetch_snapshot` subtasks since it was unclear how we could get the CDK's deletion inference mechanism would work when the collection key is `_meta/row_id`. Well, I'm pretty sure the CDK's deletion inference would still work with multiple subtasks as long as the documents and tombstones contain some discriminator field that uniquely identifies which subtask emitted them. For example with source-shopify-native, if the collection key is the compound `["_meta/store", "_meta/row_id"]` and the connector provided tombstones that had `_meta/store` pre-set, then the CDK could just updated `_meta/row_id` like it usually does & the deletion inference mechanism would ~just work~. This commit updates the CDK to support multiple `fetch_snapshot` subtasks. It includes a number of assertions to make sure connector developers provide the conditions needed for deletion inference to work.
Alex-Bair
added a commit
that referenced
this pull request
Jun 10, 2026
When introducing the capability to have multiple subtasks for a single binding in #2467, we refrained from adding support for multiple `fetch_snapshot` subtasks since it was unclear how we could get the CDK's deletion inference mechanism would work when the collection key is `_meta/row_id`. Well, I'm pretty sure the CDK's deletion inference would still work with multiple subtasks as long as the documents and tombstones contain some discriminator field that uniquely identifies which subtask emitted them. For example with source-shopify-native, if the collection key is the compound `["_meta/store", "_meta/row_id"]` and the connector provided tombstones that had `_meta/store` pre-set, then the CDK could just updated `_meta/row_id` like it usually does & the deletion inference mechanism would ~just work~. This commit updates the CDK to support multiple `fetch_snapshot` subtasks. It includes a number of assertions to make sure connector developers provide the conditions needed for deletion inference to work.
Alex-Bair
added a commit
that referenced
this pull request
Jun 11, 2026
When introducing the capability to have multiple subtasks for a single binding in #2467, we refrained from adding support for multiple `fetch_snapshot` subtasks since it was unclear how we could get the CDK's deletion inference mechanism would work when the collection key is `_meta/row_id`. Well, I'm pretty sure the CDK's deletion inference would still work with multiple subtasks as long as the documents and tombstones contain some discriminator field that uniquely identifies which subtask emitted them. For example with source-shopify-native, if the collection key is the compound `["_meta/store", "_meta/row_id"]` and the connector provided tombstones that had `_meta/store` pre-set, then the CDK could just updated `_meta/row_id` like it usually does & the deletion inference mechanism would ~just work~. This commit updates the CDK to support multiple `fetch_snapshot` subtasks. It includes a number of assertions to make sure connector developers provide the conditions needed for deletion inference to work.
Alex-Bair
added a commit
that referenced
this pull request
Jun 11, 2026
When introducing the capability to have multiple subtasks for a single binding in #2467, we refrained from adding support for multiple `fetch_snapshot` subtasks since it was unclear how we could get the CDK's deletion inference mechanism would work when the collection key is `_meta/row_id`. Well, I'm pretty sure the CDK's deletion inference would still work with multiple subtasks as long as the documents and tombstones contain some discriminator field that uniquely identifies which subtask emitted them. For example with source-shopify-native, if the collection key is the compound `["_meta/store", "_meta/row_id"]` and the connector provided tombstones that had `_meta/store` pre-set, then the CDK could just updated `_meta/row_id` like it usually does & the deletion inference mechanism would ~just work~. This commit updates the CDK to support multiple `fetch_snapshot` subtasks. It includes a number of assertions to make sure connector developers provide the conditions needed for deletion inference to work.
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
3 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:
open_bindingfunction to support dictionary-based fetch functions forfetch_changes,fetch_page, andfetch_snapshot. This allows each function to run as a separate subtask with its own independent state.source-stripe-nativeto use the CDK feature to capture data for connected accounts when enabled in the EndpointConfig.Workflow steps:
(How does one use this feature, and how has it changed)
Documentation links affected:
(list any documentation links that you created, or existing ones that you've identified as needing updates, along with a brief description)
Notes for reviewers:
(anything that might help someone review this PR)
This change is