🚨 🚨 Source Stripe: fix multiple issues regarding Refunds, CheckoutSessions and CheckoutSessionsLineItems + fix stream schemas

[davydov-d]

What

This PR addresses multiple issues described in https://github.com/airbytehq/oncall/issues/3428 that are related to incremental syncs of the Refunds, CheckoutSessions and CheckoutSessionsLineItems streams + fixes stream schemas:

• Refunds missing data in the incremental sync mode
• CheckoutSessions missing data for one day after a reset
• CheckoutSessionLineItems potential data loss
• Incremental streams with created cursor field produce duplicated data in edge cases
• Fix the default_tax_rates schema in the Invoices, Subscriptions, SubscriptionSchedule streams

How

• Do not use event-based API for the Refunds stream
• Change the cursor field for the CheckoutSessions stream
• Re-implement the CheckoutSessionsLineItems stream
• Introduce two new fields to the CheckoutSessionsLineItems stream
• Increase start datetime of a first slice by one second to avoid duplicate data
• Remove parent_id and add_parent_id of the StripeLazySubStream class
• Introduce new callable param to the stream for post-processing the records given the slice as a context
• Update stream schemas
• Update docs
• Cover these changes with unit tests

🚨 User Impact 🚨

• The CheckoutSessionLineItems and Refunds streams will have their cursor fields changed what is reflected in the migration guide

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
airbyte-docs	⬜️ Ignored (Inspect)	Visit Preview		Nov 16, 2023 3:54pm

[github-actions]

Before Merging a Connector Pull Request

Wow! What a great pull request you have here! 🎉

To merge this PR, ensure the following has been done/considered for each connector added or updated:

• PR name follows PR naming conventions
• Breaking changes are considered. If a Breaking Change is being introduced, ensure an Airbyte engineer has created a Breaking Change Plan.
• Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
• You've updated the connector's metadata.yaml file any other relevant changes, including a breakingChanges entry for major version bumps. See metadata.yaml docs
• Secrets in the connector's spec are annotated with airbyte_secret
• All documentation files are up to date. (README.md, bootstrap.md, docs.md, etc...)
• Changelog updated in docs/integrations/<source or destination>/<name>.md with an entry for the new version. See changelog example
• Migration guide updated in docs/integrations/<source or destination>/<name>-migrations.md with an entry for the new version, if the version is a breaking change. See migration guide example
• If set, you've ensured the icon is present in the platform-internal repo. (Docs)

If the checklist is complete, but the CI check is failing,

1. Check for hidden checklists in your PR description

2. Toggle the github label checklist-action-run on/off to re-run the checklist CI.

[maxi297]

Hey Denys! To speed up the review process, can you describe in which cases could these cases occur?

• Refunds missing data in the incremental sync mode
  • Are "refund.created", "refund.updated" events not enough? In which case aren’t they?
• CheckoutSessions missing data for one day after a reset
  • legacy_cursor_field has been changed from expires_at to created. My guess is that sessions expires after 24 hours and we would lose them by using expires_at as a cursor field. Can you confirm that?
• CheckoutSessionLineItems potential data loss
  • Before, the parent stream would be checkout_sessions and we would use it as full incremental. How does moving to only using checkout_sessions as incremental could prevent data loss?

[davydov-d]

@maxi297

• Refunds missing data in the incremental sync mode

  • Are "refund.created", "refund.updated" events not enough? In which case aren’t they?

We don't know the reason but the Stripe API does not return events of types refund.* and this is not a transient issue. Therefore we decided to use traditional endpoint for both sync modes of this stream. To always have the up-to-date data, customers can make use of the lookback window

• CheckoutSessions missing data for one day after a reset

  • legacy_cursor_field has been changed from expires_at to created. My guess is that sessions expires after 24 hours and we would lose them by using expires_at as a cursor field. Can you confirm that?

Yes, we would lose sessions that would be created between the time of the sync and the most recent expires_at value which is ~24 hours

• CheckoutSessionLineItems potential data loss

  • Before, the parent stream would be checkout_sessions and we would use it as full incremental. How does moving to only using checkout_sessions as incremental could prevent data loss?

First of all, having changed the parent stream's legacy cursor field, we need to change this stream's cursor field as well -- this is how we prevent the data loss. Also, we're not moving to using checkout_sessions only in incremental mode - we're using it in the same mode that checkout_sessions_line_items is used.
Second - I got rid of the additional lookback window of 1 day for this stream which was also tied to the previous cursor field.
And the last one - now we do not have to use the parent stream in semi-incremental mode when syncing line items in incremental - this must be a performance boost.

[maxi297]

Those changes LGTM. Thanks for providing more information and testing exhaustively.

I would like us to take actions on what you mentioned We don't know the reason but the Stripe API does not return events of types refund.* and this is not a transient issue:

• Can we document this in our code? It feels weird that this stream does not leverage the Events API and it feels like it deserve an explanation
• Can we open a ticket with Stripe to see why the refunds event would not work? Based on their documentation, it should and it feels like we could get more information on this API that might affect other streams we have

Once both are done and my comments are addressed, I'll approve the PR

[davydov-d]

@maxi297 the complaint to Stripe has been filed. I do not have any link, however I think they should reply in an email shortly.
Comment added, migration guide and test updated.

[maxi297]

LGTM! Thanks Denys

[davydov-d]

@katmarkham a kind reminder to review this PR

[katmarkham]

Version 5.0.0 introduces fixes for the CheckoutSessions, CheckoutSessionsLineItems and Refunds streams. The cursor field is changed for the CheckoutSessionsLineItems and Refunds streams. This will prevent data loss during incremental syncs.
Also, the Invoices, Subscriptions and SubscriptionSchedule stream schemas have been updated.

[davydov-d]

applied

[katmarkham]

   # The Refunds stream does not utilize the Events API as it created issues with data loss during the incremental syncs.
        # Therefore, we're using the regular API with the `created` cursor field. A bug has been filed with Stripe.

[davydov-d]

applied

[katmarkham]

The Lookback Window only applies to streams that do not support event-based incremental syncs: Events,
SetupAttempts, ShippingRates, BalanceTransactions, Files, FileLinks, Refunds.

[davydov-d]

applied

[katmarkham]

This stream differs from others in that it runs parent stream in exactly same sync mode it is run itself to generate stream slices.
(this is unclear to me, perhaps we can talk live?)
It also uses regular /v1 API endpoints to sync data no matter what the sync mode is. This means that the event-based API may only
be utilized by the parent stream.
(should may be "can"?)

[davydov-d]

suggestion applied: "may" changed to "can". However, this is a docstring, so, cloud users won't even see it, so I don't think this is a significant change

[katmarkham]

This change fixes multiple incremental sync issues with the Refunds, Checkout Sessions and Checkout Sessions Line Items streams:

• Refunds stream was not syncing data in the incremental sync mode. Cursor field has been updated to "created" to allow for incremental syncs. Because of the changed cursor field of the Refunds stream, incremental syncs will not reflect every update of the records that have been previously replicated. Only newly created records will be synced. To always have the up-to-date data, users are encouraged to make use of the lookback window.

• CheckoutSessions stream had been missing data for one day when using the incremental sync mode after a reset; this has been resolved.

• CheckoutSessionsLineItems previously had potential data loss. It has been updated to use a new cursor field ___.

• Incremental streams with the created cursor had been duplicating some data; this has been fixed.

Stream schema update is a breaking change as well as changing the cursor field for the Refunds and the CheckoutSessionsLineItems stream. A schema refresh and data reset of all effected streams is required after the update is applied.

Also, this update affects three more streams: Invoices, Subscriptions, SubscriptionSchedule. Schemas are changed in this update so that the declared data types would match the actual data.

[davydov-d]

suggestion applied