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 Stripe: sync modified objects #29330

Merged
merged 117 commits into from
Sep 6, 2023

Conversation

davydov-d
Copy link
Collaborator

@davydov-d davydov-d commented Aug 10, 2023

What

#15509

How

  • Refactor existing code
  • Get rid of a class-per-stream pattern
  • Implement UpdatedCurdsorIncrementalStripeStream - base class that provides incremental sync mode based on events
  • Implement few more classes to combine new incremental approach with existing usual approach:
    • full refresh and initial incremental sync use common endpoints, further incremental syncs use new approach
    • same for sub-streams
    • same for lazy sub-streams - when we try to fetch data from the parent record first
  • Implement stream selectors as a part of a strategy pattern described above
  • Implement record extractors

🚨 User Impact 🚨

  • This is a breaking change because a cursor field is changed.
  • Event based incremental syncs are limited up to 30 days - users won't be able to sync older data updates using incremental sync mode
  • Event based incremental data can not be expanded unlike in common endpoints - we need to decide how to handle this. As an option, we can potentially overcome it by querying the usual endpoint by entity id. - the data is returned expanded

davydov-d added 30 commits May 10, 2023 12:05
@davydov-d
Copy link
Collaborator Author

davydov-d commented Aug 22, 2023

This is amazing! We went from the previous graph with twice as many boxes and mostly inheritance to this graph with mostly composition:

stream

The cognitive load required for me to understand how the code works and interacts was way less as I didn't have to navigate the hierarchy tree and once I understood IncrementalStripeStream for example, I didn't have to navigate through the files to understand the other streams using it.

This wasn't easy to change and you've done tremendous work on this PR. Thanks @davydov-d !!

I bet reviewing this PR was a challenge as well, so thank you too!

@davydov-d davydov-d changed the title ✨ ✨ Source Stripe: sync modified objects 🚨 🚨 Source Stripe: sync modified objects Aug 23, 2023
@github-actions
Copy link
Contributor

source-stripe test report (commit ac129a744e) - ✅

⏲️ Total pipeline duration: 08mn42s

Step Result
Validate airbyte-integrations/connectors/source-stripe/metadata.yaml
Connector version semver check
Connector version increment check
QA checks
Code format checks
Connector package install
Build source-stripe docker image for platform linux/x86_64
Unit tests
Acceptance tests

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-stripe test

@davydov-d davydov-d requested a review from a team August 23, 2023 20:33
@davydov-d
Copy link
Collaborator Author

@github-actions
Copy link
Contributor

source-stripe test report (commit e635a3e862) - ✅

⏲️ Total pipeline duration: 08mn44s

Step Result
Connector package install
Build source-stripe docker image for platform linux/x86_64
Unit tests
Acceptance tests
Code format checks
Validate airbyte-integrations/connectors/source-stripe/metadata.yaml
Connector version semver check
Connector version increment check
QA checks

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-stripe test

@davydov-d davydov-d requested a review from evantahler August 28, 2023 09:47
… github.com:airbytehq/airbyte into ddavydov/#15509-source-stripe-sync-modified-objects
@github-actions
Copy link
Contributor

source-stripe test report (commit 6c3dfd13f0) - ❌

⏲️ Total pipeline duration: 09mn24s

Step Result
Connector package install
Build source-stripe docker image for platform linux/x86_64
Unit tests
Acceptance tests
Code format checks
Validate airbyte-integrations/connectors/source-stripe/metadata.yaml
Connector version semver check
Connector version increment check
QA checks

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-stripe test

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2023

source-stripe test report (commit dcdee0913e) - ❌

⏲️ Total pipeline duration: 10mn06s

Step Result
Connector package install
Build source-stripe docker image for platform linux/x86_64
Unit tests
Acceptance tests
Code format checks
Validate airbyte-integrations/connectors/source-stripe/metadata.yaml
Connector version semver check
Connector version increment check
QA checks

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-stripe test

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2023

source-stripe test report (commit 9d940acd93) - ❌

⏲️ Total pipeline duration: 09mn44s

Step Result
Connector package install
Build source-stripe docker image for platform linux/x86_64
Unit tests
Acceptance tests
Code format checks
Validate airbyte-integrations/connectors/source-stripe/metadata.yaml
Connector version semver check
Connector version increment check
QA checks

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-stripe test

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2023

source-stripe test report (commit 34f62c80eb) - ✅

⏲️ Total pipeline duration: 09mn46s

Step Result
Connector package install
Build source-stripe docker image for platform linux/x86_64
Unit tests
Acceptance tests
Code format checks
Validate airbyte-integrations/connectors/source-stripe/metadata.yaml
Connector version semver check
Connector version increment check
QA checks

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-stripe test

@davydov-d davydov-d merged commit 6008a1e into master Sep 6, 2023
@davydov-d davydov-d deleted the ddavydov/#15509-source-stripe-sync-modified-objects branch September 6, 2023 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation breaking-change Don't merge me unless you are ready. checklist-action-run connectors/source/stripe
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants