🎉 Source Stripe: Implement StripePartition #31696

yevhenii-ldv · 2023-10-22T19:55:14Z

What

Resolves airbytehq/airbyte-internal-issues#2147.

How

Describe the solution

Pre-merge Actions

Community member or Airbyter

Grant edit access to maintainers (instructions)
Unit & integration tests added

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
If new credentials are required for use in CI, add them to GSM. Instructions.

vercel · 2023-10-22T19:55:20Z

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 7, 2023 3:54pm

github-actions · 2023-10-22T19:55:41Z

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,

Check for hidden checklists in your PR description
Toggle the github label checklist-action-run on/off to re-run the checklist CI.

airbyte-oss-build-runner · 2023-10-22T19:57:55Z

source-stripe test report (commit `e45c5aa66c`) - ❌

⏲️ Total pipeline duration: 35.67s

Step	Result
Build source-stripe docker image for platform(s) linux/amd64	✅
Unit tests	✅
Acceptance tests	❌
Check our base image is used	❌
Code format checks	✅
Validate metadata for source-stripe	✅
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

airbyte-oss-build-runner · 2023-10-22T20:08:05Z

source-stripe test report (commit `1c091df4cc`) - ❌

⏲️ Total pipeline duration: 33.69s

Step	Result
Build source-stripe docker image for platform(s) linux/amd64	✅
Unit tests	✅
Acceptance tests	❌
Check our base image is used	❌
Code format checks	✅
Validate metadata for source-stripe	✅
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

airbyte-oss-build-runner · 2023-10-22T21:06:53Z

source-stripe test report (commit `56ed09963c`) - ❌

⏲️ Total pipeline duration: 28.05s

Step	Result
Build source-stripe docker image for platform(s) linux/amd64	✅
Unit tests	✅
Acceptance tests	❌
Check our base image is used	❌
Code format checks	✅
Validate metadata for source-stripe	✅
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

airbyte-oss-build-runner · 2023-10-22T21:10:50Z

source-stripe test report (commit `64b5167388`) - ❌

⏲️ Total pipeline duration: 24.10s

Step	Result
Build source-stripe docker image for platform(s) linux/amd64	✅
Unit tests	✅
Acceptance tests	❌
Check our base image is used	❌
Code format checks	✅
Validate metadata for source-stripe	✅
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

airbyte-oss-build-runner · 2023-10-22T21:16:50Z

source-stripe test report (commit `1c091df4cc`) - ❌

⏲️ Total pipeline duration: 01mn04s

Step	Result
Build source-stripe docker image for platform(s) linux/amd64	✅
Unit tests	✅
Acceptance tests	❌
Check our base image is used	❌
Code format checks	✅
Validate metadata for source-stripe	✅
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

airbyte-oss-build-runner · 2023-10-22T22:49:38Z

source-stripe test report (commit `f568b03dfb`) - ❌

⏲️ Total pipeline duration: 30.76s

Step	Result
Build source-stripe docker image for platform(s) linux/amd64	✅
Unit tests	✅
Acceptance tests	❌
Check our base image is used	❌
Code format checks	✅
Validate metadata for source-stripe	✅
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

girarda · 2023-10-23T20:34:35Z

airbyte-cdk/python/airbyte_cdk/sources/streams/concurrent/thread_based_concurrent_stream.py

@@ -168,7 +168,10 @@ def as_airbyte_stream(self) -> AirbyteStream:

        keys = self._primary_key
        if keys and len(keys) > 0:
-            stream.source_defined_primary_key = [keys]
+            if isinstance(keys, str):


did you encounter a case where keys was a string and not a list? This wouldn't respect the interface

Yep, agree, will fix

girarda · 2023-10-23T20:35:42Z

airbyte-integrations/connectors/source-stripe/source_stripe/partition.py

@@ -0,0 +1,130 @@
+#


can you unit test these classes

girarda · 2023-10-23T20:58:06Z

airbyte-integrations/connectors/source-stripe/source_stripe/source.py

@@ -158,10 +220,203 @@ def streams(self, config: Mapping[str, Any]) -> List[Stream]:
            ],
            **args,
        )
-        return [
+
+        concurrent_streams = [


we should only create concurrent streams when running in full refresh. You can tell the source will run in full refresh mode by checking that there's no input state file. Here's an example

girarda · 2023-10-23T21:01:00Z

airbyte-integrations/connectors/source-stripe/source_stripe/source.py

+                cursor_value="{{ last_records[-1]['id'] if last_records else None }}",
+                config={},
+                parameters={},
+                page_size=1000,


let's make this a constant

girarda · 2023-10-23T21:01:26Z

airbyte-integrations/connectors/source-stripe/source_stripe/source.py

+        concurrent_stream = StreamFacade(
+            ThreadBasedConcurrentStream(
+                partition_generator=StripePartitionGenerator(base_stream, self.message_repository, paginated_requester),
+                max_workers=1,


let's make this a constant

girarda · 2023-10-23T21:23:57Z

airbyte-integrations/connectors/source-stripe/source_stripe/partition.py

+        self._requester = requester
+
+    @property
+    def request_parameters(self) -> Optional[Mapping[str, Any]]:


we shouldn't expose these properties publicly for arbitrary source because not all sources are HTTP API sources

girarda · 2023-10-23T21:26:04Z

airbyte-integrations/connectors/source-stripe/source_stripe/partition.py

+    @property
+    def request_parameters(self) -> Mapping[str, Any]:
+        params = {
+            "limit": 100,


this parameter should be set by the paginator

girarda · 2023-10-23T21:36:56Z

airbyte-integrations/connectors/source-stripe/source_stripe/partition.py

+class SourcePartition(Partition):
+    def __init__(self, _slice: Mapping[str, Any], message_repository: MessageRepository, requester: PaginatedRequester):
+        self._slice = _slice
+        self._message_repository = message_repository


why is the message repository needed? it doesn't seem used

girarda · 2023-10-23T21:40:04Z

airbyte-integrations/connectors/source-stripe/source_stripe/partition.py

+    def __init__(self, requester: Requester, record_selector: RecordSelector, paginator: DefaultPaginator):
+        self._requester = requester
+        self._record_selector = record_selector
+        self._page_token_option = paginator.page_token_option


why create a new field instead of always referring to paginator.page_token_option?

…gy and <Source>PaginationStrategy

yevhenii-ldv · 2023-10-31T20:31:09Z

airbyte-cdk/python/airbyte_cdk/sources/declarative/parsers/class_types_registry.py

-    SessionTokenAuthenticator,
+    LegacySessionTokenAuthenticator,


@girarda It seems that when was changed the SessionTokenAuthenticator to LegacySessionTokenAuthenticator, wasn't changed this file, so mypy complained about this one. Let me know if this is not the case

interesting. would you mind fixing this in a separate PR? no reason to delay this fix with the stripe partition work

sure, I've created separate PR for this

girarda · 2023-10-31T23:22:12Z

airbyte-cdk/python/airbyte_cdk/sources/declarative/parsers/class_types_registry.py

@@ -56,7 +56,7 @@
 """
 CLASS_TYPES_REGISTRY contains a mapping of developer-friendly string -> class to abstract the specific class referred to
 """
-CLASS_TYPES_REGISTRY: Mapping[str, Type] = {
+CLASS_TYPES_REGISTRY: Mapping[str, type] = {


why this change?

When I make changes to this file, mypy finds the following error:

airbyte_cdk/sources/declarative/parsers/class_types_registry.py:59: error: Missing type parameters for generic type "Type" [type-arg]

I've created a separate PR to fix the SessionTokenAuthorization, and you can see there that mypy is failing with this error: https://github.com/airbytehq/airbyte/actions/runs/6719622494/job/18261600772?pr=32040

It seems that when using a Type (imported from the typing), we also need to specify a parameter in the annotation (Type[<param>]).

Ours doesn't specify a parameter, so I replaced it with a generic type to get around the mypy error.

girarda · 2023-10-31T23:30:54Z

...rbyte_cdk/sources/declarative/requesters/paginators/strategies/cursor_pagination_strategy.py

-from airbyte_cdk.sources.declarative.types import Config
+from airbyte_cdk.sources.declarative.types import Config, Record
+
+DECODED_RESPONSE_TYPE = Union[Mapping[str, Any], List[dict[str, Any]]]


this is needed because the decoder can return either a mapping or a list, is this correct? If so, can you update the decoder to return a value of this type to document why this is needed

girarda · 2023-10-31T23:36:21Z

...rbyte_cdk/sources/declarative/requesters/paginators/strategies/cursor_pagination_strategy.py

        return token if token else None

-    def reset(self):
+    @abstractmethod
+    def stop(self, response: DECODED_RESPONSE_TYPE, headers: Mapping[str, Any], last_records: List[Record]) -> bool:


Why do we need both a stop_condition and a stop method? Maybe we can remove the stop_condition property and only keep the private field in the LowCodeCursorPaginationStrategy

…/source-stripe/implement-partition

…package' of github.com:airbytehq/airbyte into ykurochkin/source-stripe/implement-partition # Conflicts: # airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/paginators/strategies/__init__.py # airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/paginators/strategies/cursor_pagination_strategy.py

…package' of github.com:airbytehq/airbyte into ykurochkin/source-stripe/implement-partition # Conflicts: # airbyte-integrations/connectors/source-stripe/source_stripe/source.py # docs/integrations/sources/stripe.md

…package' of github.com:airbytehq/airbyte into ykurochkin/source-stripe/implement-partition

girarda · 2023-11-05T18:36:28Z

airbyte-integrations/connectors/source-stripe/setup.py

@@ -5,7 +5,7 @@

 from setuptools import find_packages, setup

-MAIN_REQUIREMENTS = ["airbyte-cdk==0.52.8", "stripe==2.56.0", "pendulum==2.1.2"]
+MAIN_REQUIREMENTS = ["airbyte-cdk>=0.52.8", "stripe==2.56.0", "pendulum==2.1.2"]


let's pin the version before releasing to avoid the connector accidentally breaking if we make a breaking change

girarda · 2023-11-05T18:37:13Z

airbyte-integrations/connectors/source-stripe/source_stripe/source.py

+            return None
+
+
+class StripePaginationStrategy(CursorPaginationStrategy):


let's write unit tests for this class

girarda · 2023-11-05T18:37:31Z

airbyte-integrations/connectors/source-stripe/source_stripe/source.py

+
+
+class StripePaginator(DefaultPaginator):
+    def path(self) -> Optional[str]:


let's write unit tests for this method

…package' of github.com:airbytehq/airbyte into ykurochkin/source-stripe/implement-partition

lazebnyi · 2024-02-08T19:00:18Z

Hey @yevhenii-ldv
Are you still in work on this pull request?

Implement Stripe Partition

e45c5aa

yevhenii-ldv requested a review from keu October 22, 2023 19:55

octavia-squidington-iii added area/connectors Connector related issues connectors/source/stripe labels Oct 22, 2023

octavia-squidington-iv requested a review from a team October 22, 2023 19:56

Update version and docs

1c091df

octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Oct 22, 2023

yevhenii-ldv removed the request for review from a team October 22, 2023 20:06

vercel bot deployed to Preview October 22, 2023 20:07 View deployment

octavia-squidington-iv requested a review from a team October 22, 2023 20:07

yevhenii-ldv requested a review from a team as a code owner October 22, 2023 21:04

octavia-squidington-iii added the CDK Connector Development Kit label Oct 22, 2023

yevhenii-ldv force-pushed the ykurochkin/source-stripe/implement-partition branch from 56ed099 to 64b5167 Compare October 22, 2023 21:08

yevhenii-ldv force-pushed the ykurochkin/source-stripe/implement-partition branch from 64b5167 to 1c091df Compare October 22, 2023 21:13

octavia-squidington-iii removed the CDK Connector Development Kit label Oct 22, 2023

Update source_defined_primary_key logic for ThreadBasedConcurrentStream

f568b03

octavia-squidington-iii added the CDK Connector Development Kit label Oct 22, 2023

yevhenii-ldv requested review from girarda and removed request for a team October 22, 2023 23:54

girarda reviewed Oct 23, 2023

View reviewed changes

octavia-squidington-iv requested a review from a team October 23, 2023 21:41

Merge branch 'master' into ykurochkin/source-stripe/implement-partition

3686904

girarda requested a review from brianjlai October 27, 2023 05:38

yevhenii-ldv added 2 commits October 27, 2023 11:35

Little updates

c836834

Splitting CursorPaginationStrategy into LowCodeCursorPaginationStrate…

93b34c5

…gy and <Source>PaginationStrategy

octavia-squidington-iii added the CDK Connector Development Kit label Oct 31, 2023

vercel bot deployed to Preview October 31, 2023 16:21 View deployment

yevhenii-ldv commented Oct 31, 2023

View reviewed changes

girarda reviewed Oct 31, 2023

View reviewed changes

yevhenii-ldv mentioned this pull request Nov 2, 2023

CDK: Fix class SessionTokenAuthenticator for CLASS_TYPES_REGISTRY mapper #32040

Merged

yevhenii-ldv added 3 commits November 2, 2023 11:17

Merge branch 'master' of github.com:airbytehq/airbyte into ykurochkin…

a7bd8dd

…/source-stripe/implement-partition

Update after cdk changed

1872989

yevhenii-ldv force-pushed the ykurochkin/source-stripe/implement-partition branch from 1ff33ee to 1872989 Compare November 2, 2023 11:40

yevhenii-ldv changed the base branch from master to ykurochkin/cdk/decoupling-the-objects-from-declarative-package November 2, 2023 11:41

yevhenii-ldv added 2 commits November 2, 2023 19:41

Add StripePaginator

468ed26

octavia-squidington-iii removed the CDK Connector Development Kit label Nov 2, 2023

yevhenii-ldv added 3 commits November 3, 2023 17:01

Merge branch 'ykurochkin/cdk/decoupling-the-objects-from-declarative-…

f7bf955

…package' of github.com:airbytehq/airbyte into ykurochkin/source-stripe/implement-partition

Merge branch 'ykurochkin/cdk/decoupling-the-objects-from-declarative-…

d56902d

…package' of github.com:airbytehq/airbyte into ykurochkin/source-stripe/implement-partition

Change HttpRequester with SourceHttpRequester

1ba98c2

girarda reviewed Nov 5, 2023

View reviewed changes

octavia-squidington-iv requested a review from a team November 5, 2023 18:44

yevhenii-ldv added 3 commits November 7, 2023 12:28

Merge branch 'ykurochkin/cdk/decoupling-the-objects-from-declarative-…

2619a23

…package' of github.com:airbytehq/airbyte into ykurochkin/source-stripe/implement-partition

Add tests

ee83ece

Bump version

230e243

keu mentioned this pull request Nov 8, 2023

Enable client-side rate limiting on source-stripe #31512 #32284

Merged

lazebnyi requested a review from davydov-d November 10, 2023 09:30

katmarkham requested review from lazebnyi and removed request for a team November 27, 2023 15:20

girarda closed this May 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🎉 Source Stripe: Implement StripePartition #31696

🎉 Source Stripe: Implement StripePartition #31696

yevhenii-ldv commented Oct 22, 2023

vercel bot commented Oct 22, 2023 •

edited

Loading

github-actions bot commented Oct 22, 2023 •

edited by yevhenii-ldv

Loading

airbyte-oss-build-runner commented Oct 22, 2023

airbyte-oss-build-runner commented Oct 22, 2023

airbyte-oss-build-runner commented Oct 22, 2023

airbyte-oss-build-runner commented Oct 22, 2023

airbyte-oss-build-runner commented Oct 22, 2023

airbyte-oss-build-runner commented Oct 22, 2023

girarda Oct 23, 2023

yevhenii-ldv Oct 24, 2023

girarda Oct 23, 2023

yevhenii-ldv Nov 2, 2023

girarda Oct 23, 2023

girarda Oct 23, 2023

girarda Oct 23, 2023

girarda Oct 23, 2023

girarda Oct 23, 2023

girarda Oct 23, 2023

girarda Oct 23, 2023

yevhenii-ldv Oct 31, 2023

girarda Oct 31, 2023

yevhenii-ldv Nov 1, 2023 •

edited

Loading

girarda Oct 31, 2023

yevhenii-ldv Nov 1, 2023

girarda Oct 31, 2023

girarda Oct 31, 2023

girarda Nov 5, 2023

girarda Nov 5, 2023

girarda Nov 5, 2023

lazebnyi commented Feb 8, 2024

		return None


		class StripePaginationStrategy(CursorPaginationStrategy):



		class StripePaginator(DefaultPaginator):
		def path(self) -> Optional[str]:

🎉 Source Stripe: Implement StripePartition #31696

🎉 Source Stripe: Implement StripePartition #31696

Conversation

yevhenii-ldv commented Oct 22, 2023

What

How

Recommended reading order

Pre-merge Actions

Community member or Airbyter

Airbyter

vercel bot commented Oct 22, 2023 • edited Loading

github-actions bot commented Oct 22, 2023 • edited by yevhenii-ldv Loading

Before Merging a Connector Pull Request

airbyte-oss-build-runner commented Oct 22, 2023

source-stripe test report (commit e45c5aa66c) - ❌

airbyte-oss-build-runner commented Oct 22, 2023

source-stripe test report (commit 1c091df4cc) - ❌

airbyte-oss-build-runner commented Oct 22, 2023

source-stripe test report (commit 56ed09963c) - ❌

airbyte-oss-build-runner commented Oct 22, 2023

source-stripe test report (commit 64b5167388) - ❌

airbyte-oss-build-runner commented Oct 22, 2023

source-stripe test report (commit 1c091df4cc) - ❌

airbyte-oss-build-runner commented Oct 22, 2023

source-stripe test report (commit f568b03dfb) - ❌

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yevhenii-ldv Nov 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lazebnyi commented Feb 8, 2024

vercel bot commented Oct 22, 2023 •

edited

Loading

github-actions bot commented Oct 22, 2023 •

edited by yevhenii-ldv

Loading

source-stripe test report (commit `e45c5aa66c`) - ❌

source-stripe test report (commit `1c091df4cc`) - ❌

source-stripe test report (commit `56ed09963c`) - ❌

source-stripe test report (commit `64b5167388`) - ❌

source-stripe test report (commit `1c091df4cc`) - ❌

source-stripe test report (commit `f568b03dfb`) - ❌

yevhenii-ldv Nov 1, 2023 •

edited

Loading