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

source-braintree-native: new connector #2177

Merged
merged 3 commits into from
Dec 4, 2024
Merged

Conversation

Alex-Bair
Copy link
Contributor

@Alex-Bair Alex-Bair commented Dec 3, 2024

Description:

This is a minimal connector for Braintree. All incremental streams only capture incremental creates (not updates) due to limitations with Braintree's APIs, so regular backfills are required to capture updates. I did not include logic to trigger backfills within the connector since we already have a separate mechanism for scheduled backfills.

Braintree provides SDKs and a GraphQL API for retrieving data. Instead of using the GraphQL API and figuring out the quirks of using it for incremental replication, I decided to use the Python SDK to speed up development. This circumvents error handling baked into the Estuary CDK's HTTPMixin object, so we may need to improve error handling later.

Braintree's SDK returns custom objects instead of dictionaries, and the braintree_object_to_dict function converts these custom objects to dictionaries to feed the Pydantic models. This function may need additional handling/improvement once the connector is used in production and we better understand what types users want.

The credit_card_verifications stream is not included in the capture snapshot because I couldn't figure out how to create these in a sandbox Braintree account.

Workflow steps:

(How does one use this feature, and how has it changed)

Documentation links affected:

Documentation will need created for this connector. It should probably clarify that incremental streams only capture creates, not updates, and regular backfills are needed to capture updates. The docs PR will be linked here once it's ready.

Notes for reviewers:

Tested on a local stack. Confirmed:

  • Streams capture data & associated collections can be materialized into PostgreSQL.
  • Snapshots test pass.
  • Invalid credentials throw an error before task creation.
  • Objects returned by Braintree's SDK are converted into Pydantic models appropriately.

I could not figure out how to create a credit_card_verification object within Braintree, so I could not confirm that stream captures data.

Supabase, Strapi, and Short.io will need updated to list this new connector in our catalog.


This change is Reviewable

@Alex-Bair Alex-Bair force-pushed the bair/source-braintree branch from a943b38 to b9db8ba Compare December 3, 2024 20:32
This is a minimal connector for Braintree. All incremental streams only
capture incremental creations (not updates), so regular backfills are
required to capture updates. I did not include logic to trigger
backfills within the connector since we already have a separate
mechanism for this.

Braintree provides SDKs and a GraphQL API for retrieving data. Instead
of using the GraphQL API and figuring out the quirks of using it to
capture data, I decided to use the Python SDK. This circumvents error
handling baked into the Estuary CDK's HTTPMixin object, so we may need
to improve error handling later.

Braintree's SDK returns custom objects instead of dictionaries, and the
`braintree_object_to_dict` function converts these custom objects to
dictionaries to feed the Pydantic models. This function may need
additional handling/improvement once the connector is used in production
and we understand better what types users want.

The `credit_card_verifications` stream is not included in the capture
snapshot because I couldn't figure out how to create these in a sandbox
Braintree account.
@Alex-Bair Alex-Bair changed the title source-braintree: new connector source-braintree-native: new connector Dec 3, 2024
@Alex-Bair Alex-Bair force-pushed the bair/source-braintree branch from b9db8ba to f24c8e7 Compare December 3, 2024 20:41
@Alex-Bair Alex-Bair linked an issue Dec 3, 2024 that may be closed by this pull request
@Alex-Bair Alex-Bair added the change:planned This is a planned change label Dec 3, 2024
@Alex-Bair Alex-Bair requested a review from a team December 3, 2024 20:47
@Alex-Bair Alex-Bair marked this pull request as ready for review December 3, 2024 20:47
Copy link
Member

@willdonnelly willdonnelly left a comment

Choose a reason for hiding this comment

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

Looking good overall! I haven't dug very deeply into this, but I gave it a quick read-through and had a few questions, mostly around how search result limits are handled.

Copy link
Member

@willdonnelly willdonnelly left a comment

Choose a reason for hiding this comment

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

LGTM % one nit (which I think was just an oversight when refactoring a bunch of nearly-identical functions?)

source-braintree-native/source_braintree_native/api.py Outdated Show resolved Hide resolved
The window size used for incremental streams is now a config setting,
and hitting search limits will cause the connector to error out & state
that a smaller window size should be used.

The various incremental functions in `resources.py` have been refactored.
I did not refactor the various `fetch_foobar` functions in `api.py`
since we may need to customize the behavior based on user feedback & I
anticipate we'll need to add more streams in the near future. I think
it'll be easier/more efficient to refactor these functions when we're
done adding additional streams.
@Alex-Bair Alex-Bair force-pushed the bair/source-braintree branch from 209c978 to e8e4afe Compare December 4, 2024 19:36
@Alex-Bair Alex-Bair merged commit f98f7be into main Dec 4, 2024
72 of 79 checks passed
@Alex-Bair Alex-Bair deleted the bair/source-braintree branch December 4, 2024 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change:planned This is a planned change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

source-braintree: import to Estuary python CDK
2 participants