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: run synchronous API calls in separate threads #2195

Merged
merged 2 commits into from
Dec 11, 2024

Conversation

Alex-Bair
Copy link
Contributor

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

Description:

The Braintree SDK makes synchronous HTTP requests, which was blocking the main thread and preventing separate streams from sending concurrent API calls. Any HTTP requests made by the Braintree SDK are now wrapped in asyncio.to_thread, which runs the function in a separate thread. This unblocks the main thread and lets separate streams make concurrent API calls.

The maximum_size property that indicates how many results are found in a specific date window is now used to throw search limit errors sooner instead of after we iterate through & count up all results.

Note: The disputes stream cannot use the maximum_size property because the Braintree SDK returns a PaginatedCollection instead of a ResourceCollection for disputes, and PaginatedCollections don't have a maximum_size property.

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:

Tested on a local stack. Confirmed:

  • HTTP requests made by the Braintree SDK are done in separate threads, and separate streams are able to make concurrent API calls.
  • Search limit errors are still raised when a search limit is exceeded.

This change is Reviewable

The Braintree SDK make synchronous HTTP requests, which was blocking the
main thread and preventing separate streams from sending concurrent API
calls.

Any HTTP requests made by the Braintree SDK are now wrapped in
`asyncio.to_thread`, which runs the function in a separate thread. This
unblocks the main thread and lets separate streams make concurrent API
calls.
…rough results

The Braintree SDK exposes a `maximum_size` property for
`ResourceCollection`s that indicate how many results were found in a
specific date window. This can be used to throw search limit errors
sooner.

The `disputes` stream cannot use the `maximum_size` property because the
Braintree SDK returns a `PaginatedCollection` instead of a
`ResourceCollection` for `disputes`, and `PaginatedCollection`s don't
have a `maximum_size` property.

Note: Braintree states that `maximum_size` is an approximation due to
race conditions between the first API call to get all matching resource
IDs and the subsequent API calls to paginate through the actual
resources; if a resource no longer matches the original seach criteria
during the subsequent calls, it won't be returned. However, this
shouldn't be an issue for the connector since our search criteria is
only based on the `created_at` field, and I don't anticipate that field
would change after being set.
@Alex-Bair Alex-Bair added the change:unplanned Unplanned change, useful for things like doc updates label Dec 11, 2024
@Alex-Bair Alex-Bair marked this pull request as ready for review December 11, 2024 16:09
Copy link
Member

@williamhbaker williamhbaker left a comment

Choose a reason for hiding this comment

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

LGTM

@Alex-Bair Alex-Bair merged commit f5d43b2 into main Dec 11, 2024
73 of 79 checks passed
@Alex-Bair Alex-Bair deleted the bair/source-braintree-native-async-wrapper branch December 11, 2024 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change:unplanned Unplanned change, useful for things like doc updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants