-
Notifications
You must be signed in to change notification settings - Fork 5k
feat(source-stripe): Add optional config parameters for performance optimization #72393
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
base: master
Are you sure you want to change the base?
feat(source-stripe): Add optional config parameters for performance optimization #72393
Conversation
…ptimization - Add balance_transaction_type filter to filter balance transactions by type - Add expand_charge_refunds option to control refund expansion in charges stream - Add expand_invoice_details option to control discount/tax rate expansion in invoices stream - Add subscription_status option to filter subscriptions by status These options allow users to reduce API response sizes and improve sync performance by only fetching the data they need. Co-Authored-By: [email protected] <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksPR Slash CommandsAirbyte Maintainers (that's you!) can execute the following slash commands on your PR:
📚 Show Repo GuidanceHelpful Resources
|
|
/bump-version
|
|
Deploy preview for airbyte-docs ready! ✅ Preview Built with commit e779173. |
Only keep valid Stripe API filter values: all, canceled, ended Co-Authored-By: [email protected] <[email protected]>
|
| $ref: "#/definitions/base_requester" | ||
| request_parameters: | ||
| expand[]: '{{["data.discounts", "data.total_tax_amounts.tax_rate"]}}' | ||
| expand[]: '{{ ["data.discounts", "data.total_tax_amounts.tax_rate"] if config.get("expand_invoice_details", true) else [] }}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you link the correct API Docs in the PR description for the Event-based endpoint? Also are we sure that leaving expand[]: [] will have the correct intended behavior?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good questions!
-
Event-based endpoint API docs: The invoices incremental stream uses the Events API with
types[]filter for invoice events. The expand parameters only apply to the full refresh stream which uses the Invoices List API. I'll update the PR description to clarify this distinction. -
Empty
expand[]: []behavior: Looking at the Stripe API behavior, when you pass an empty array for expand, it's equivalent to not passing the expand parameter at all - Stripe will return the default response without expanded objects. However, I should verify this doesn't cause issues with the CDK's request parameter handling.
Let me verify the empty array behavior is handled correctly by the declarative framework. If there are concerns, we could alternatively use a conditional that omits the parameter entirely when disabled rather than passing an empty array.
…d_stream pattern) The balance_transaction_type filter could not be implemented with the created_stream pattern without breaking manifest validation. Keeping the other 3 improvements: subscription_status, expand_charge_refunds, and expand_invoice_details. Co-Authored-By: [email protected] <[email protected]>
|
I ran tests with the Stripe test credentials to verify the empty Test 1: Jinja Template Evaluation The Jinja template correctly evaluates to an empty array when disabled, and defaults to the expand values when not specified. Test 2: Stripe API Behavior
The Stripe API handles empty expand arrays gracefully - it's equivalent to not passing the parameter at all. Test 3: CDK Request Parameter Handling Note: I had to remove the
|
|
/ai-docs-review
|
Documentation ReviewOverall Assessment: The connector implementation correctly matches the Stripe API documentation, and the changelog entry is present. However, the new configuration parameters are not documented in the user-facing setup guide, which means users won't discover these performance optimization options. High Priority (Correctness Issues)No correctness issues found. The implementation aligns with Stripe API documentation:
Completeness Gaps
Readability/OrganizationDocumentation is well-organized. The changelog entry follows the existing format. Style NitsNo significant style issues. The spec descriptions in Summary of Recommended Changes
This review was generated by Devin. View session |
| If you are unsure of which value to use, we recommend leaving this setting at its default value of 365 days. | ||
|
|
||
| 10. Click **Set up source** and wait for the tests to complete. | ||
| 10. (Optional) For **Subscription Status Filter**, you can filter which subscriptions are synced. By default, all subscriptions are synced (including canceled). You can choose to sync only `canceled` or `ended` subscriptions to reduce data volume. See the [Stripe Subscriptions API](https://docs.stripe.com/api/subscriptions/list) for details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[markdownlint] reported by reviewdog 🐶
MD029/ol-prefix Ordered list item prefix [Expected: 6; Actual: 10; Style: 1/2/3]
| 10. Click **Set up source** and wait for the tests to complete. | ||
| 10. (Optional) For **Subscription Status Filter**, you can filter which subscriptions are synced. By default, all subscriptions are synced (including canceled). You can choose to sync only `canceled` or `ended` subscriptions to reduce data volume. See the [Stripe Subscriptions API](https://docs.stripe.com/api/subscriptions/list) for details. | ||
|
|
||
| 11. (Optional) For **Expand Charge Refunds**, you can disable this to exclude full refund objects from the charges stream, reducing response payload sizes. When disabled, only refund IDs are included. This setting only affects full refresh syncs. See the [Stripe Charges API](https://docs.stripe.com/api/charges/list) for details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[markdownlint] reported by reviewdog 🐶
MD029/ol-prefix Ordered list item prefix [Expected: 7; Actual: 11; Style: 1/2/3]
|
|
||
| 11. (Optional) For **Expand Charge Refunds**, you can disable this to exclude full refund objects from the charges stream, reducing response payload sizes. When disabled, only refund IDs are included. This setting only affects full refresh syncs. See the [Stripe Charges API](https://docs.stripe.com/api/charges/list) for details. | ||
|
|
||
| 12. (Optional) For **Expand Invoice Details**, you can disable this to exclude full discount and tax rate objects from the invoices stream, reducing response payload sizes. When disabled, only IDs are included. This setting only affects full refresh syncs. See the [Stripe Invoices API](https://docs.stripe.com/api/invoices/list) for details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[markdownlint] reported by reviewdog 🐶
MD029/ol-prefix Ordered list item prefix [Expected: 8; Actual: 12; Style: 1/2/3]
|
|
||
| 12. (Optional) For **Expand Invoice Details**, you can disable this to exclude full discount and tax rate objects from the invoices stream, reducing response payload sizes. When disabled, only IDs are included. This setting only affects full refresh syncs. See the [Stripe Invoices API](https://docs.stripe.com/api/invoices/list) for details. | ||
|
|
||
| 13. Click **Set up source** and wait for the tests to complete. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[markdownlint] reported by reviewdog 🐶
MD029/ol-prefix Ordered list item prefix [Expected: 9; Actual: 13; Style: 1/2/3]
What
Adds optional configuration parameters to the Stripe connector that allow users to filter data and reduce API response sizes for improved sync performance. This addresses performance concerns for accounts with large data volumes.
How
Added 3 new optional configuration parameters to the connector spec and updated the corresponding stream definitions:
subscription_status- Filter subscriptions by status (default:all)all,canceled,endedexpand_charge_refunds- Toggle whether to expand refund objects in charges (default:true)expand_invoice_details- Toggle whether to expand discount and tax rate objects in invoices (default:true)All defaults maintain backward compatibility with existing behavior.
Important note on expand parameters: The
expand_charge_refundsandexpand_invoice_detailsoptions only affect full refresh syncs which use the direct List APIs. Incremental syncs use the Events API which returns full objects without expand parameters.Review guide
manifest.yaml- spec section (lines ~1902-1931): New config parameter definitions with ENUMsmanifest.yaml-subscriptionsstream (line ~359): Made status configurablemanifest.yaml-invoicesstream (line ~389): Made expand conditionalmanifest.yaml-chargesstream (line ~523): Made expand conditionaldocs/integrations/sources/stripe.md- Setup guide steps 10-12 and "Performance optimization settings" sectionRecommended reviewer verification:
expand[]: [](empty array) when expand options are disabled is handled correctly by the CDKsubscription_statusmatch current Stripe API documentationUpdates since last revision
subscription_statusenum to only include valid Stripe API filter values (all,canceled,ended). Removed invalid values that were subscription object statuses rather than API filter parameters.balance_transaction_typeconfig option - it was incompatible with thecreated_streampattern used by thebalance_transactionsstream and caused manifest validation errors.User Impact
Users can now optionally configure these parameters to reduce sync times and data transfer:
canceledorendedsubscriptions)No negative impact on existing users - all defaults preserve current behavior.
Can this PR be safely reverted and rolled back?
Requested by @agarctfi
Link to Devin run: https://app.devin.ai/sessions/ae559260b9e74cb8bcefe6d174e758af