low-code: Yield records from generators instead of keeping them in in-memory lists #36406

girarda · 2024-03-22T21:45:23Z

What

Improve memory usage by yielding records from generators instead of returning lists of objects

This PR addresses a part of https://github.com/airbytehq/airbyte-internal-issues/issues/6554

How

Update the record selector, extractor, and filter interfaces to work on generators instead of lists of records
Update the paginator interface to only use the number of records read and the last record instead of the full list of records read
Update the simple retriever to tie in everything together

Reading order

airbyte-cdk/python/airbyte_cdk/sources/declarative/extractors/record_extractor.py
airbyte-cdk/python/airbyte_cdk/sources/declarative/extractors/http_selector.py
airbyte-cdk/python/airbyte_cdk/sources/declarative/extractors/dpath_extractor.py
airbyte-cdk/python/airbyte_cdk/sources/declarative/extractors/record_selector.py
airbyte-cdk/python/airbyte_cdk/sources/declarative/extractors/record_filter.py
airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/paginators/paginator.py
airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/paginators/no_pagination.py
airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/paginators/default_paginator.py
airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/paginators/strategies/pagination_strategy.py
airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/paginators/strategies/offset_increment.py
airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/paginators/strategies/page_increment.py
airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/paginators/strategies/cursor_pagination_strategy.py
airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/paginators/strategies/stop_condition.py
airbyte-cdk/python/airbyte_cdk/sources/declarative/retrievers/simple_retriever.py
airbyte-cdk/python/airbyte_cdk/sources/declarative/retrievers/simple_retriever.py

vercel · 2024-03-22T21:45:29Z

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		May 14, 2024 11:02pm

…m:airbytehq/airbyte into alex/replace_last_records_from_paginators

girarda · 2024-03-26T18:17:40Z

airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/paginators/default_paginator.py

@@ -219,7 +223,7 @@ def get_request_body_data(
        stream_state: Optional[StreamState] = None,
        stream_slice: Optional[StreamSlice] = None,
        next_page_token: Optional[Mapping[str, Any]] = None,
-    ) -> Optional[Union[Mapping[str, Any], str]]:
+    ) -> Union[Mapping[str, Any], str]:


this never returns None

girarda · 2024-03-26T18:17:47Z

airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/paginators/default_paginator.py

@@ -228,7 +232,7 @@ def get_request_body_json(
        stream_state: Optional[StreamState] = None,
        stream_slice: Optional[StreamSlice] = None,
        next_page_token: Optional[Mapping[str, Any]] = None,
-    ) -> Optional[Mapping[str, Any]]:
+    ) -> Mapping[str, Any]:


this never returns None

artem1205

LGTM!

This reverts commit 5fe60b7.

girarda · 2024-05-10T04:32:47Z

confirmed this change in combination with some changes on the iterable side helps with the memory usage of the connector.

The large spikes show attempts with all the fixes, and the last (much lower one) shows the memory usage with this change, using generators in the custom components, and reducing the size of the time windows:

The underlying issue is that iterable returns gigantic responses (I've seen one ~4GB). I think fixing this would require streaming the responses, which is out of scope for this PR

…8026)"" This reverts commit f75fb47.

….utc (#38026)""" This reverts commit 5ce3693.

This reverts commit c7b94f9.

…datetime.utc (#38026)"""" This reverts commit 43983ed.

brianjlai

changes look good to me, but i suspect there will be some merge conflicts since it looks like you are fixing some mypy errors on certain files that i also fixed in one of my PRs that I merged

brianjlai · 2024-05-13T21:21:27Z

airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/paginators/default_paginator.py

        if self.page_size_option and self.pagination_strategy.get_page_size() and self.page_size_option.inject_into == option_type:
-            options[self.page_size_option.field_name.eval(config=self.config)] = self.pagination_strategy.get_page_size()
+            options[self.page_size_option.field_name.eval(config=self.config)] = self.pagination_strategy.get_page_size()  # type: ignore # field_name is known to be an interpolated string


sigh i hate these so much

brianjlai · 2024-05-13T21:24:27Z

airbyte-cdk/python/airbyte_cdk/sources/declarative/retrievers/simple_retriever.py

@@ -64,7 +64,8 @@ class SimpleRetriever(Retriever):
    def __post_init__(self, parameters: Mapping[str, Any]) -> None:
        self._paginator = self.paginator or NoPagination(parameters=parameters)
        self._last_response: Optional[requests.Response] = None
-        self._records_from_last_response: List[Record] = []
+        self._last_page_size: int = 0


discussed a bit w/ Alex over Slack, since we yield from the previous set of records before calculating the next_page_token, we should have the up to date count of records when we pass self._last_page_size to the paginator

girarda added 2 commits March 22, 2024 14:00

yield from generators

3110caa

update interfaces and fix the tests

10640ab

octavia-squidington-iii added the CDK Connector Development Kit label Mar 22, 2024

girarda added 12 commits March 22, 2024 15:15

fix some mypy issues

148419e

fix the rest of mypy issues

3a7a0b8

update the schema

09e8696

Add last_page_size and last_record to pagination context

30ba4d4

boundaries check

97449e6

add a test

a7b57dd

format

ea63f36

Merge branch 'master' into alex/replace_last_records_from_paginators

d89dcbe

missing unit test

2aaa8d4

Merge branch 'alex/replace_last_records_from_paginators' of github.co…

4e193d3

…m:airbytehq/airbyte into alex/replace_last_records_from_paginators

missing newline

7b514e0

merge

855e77f

girarda changed the base branch from master to alex/replace_last_records_from_paginators March 26, 2024 18:10

vercel bot deployed to Preview March 26, 2024 18:14 View deployment

girarda commented Mar 26, 2024

View reviewed changes

girarda mentioned this pull request Mar 26, 2024

low-code: Add last_page_size and last_record to pagination context #36408

Merged

girarda added 2 commits March 26, 2024 11:24

fix imports

834fc96

update unit tests

341d671

Base automatically changed from alex/replace_last_records_from_paginators to master April 2, 2024 17:43

tolik0 approved these changes May 3, 2024

View reviewed changes

artem1205 approved these changes May 3, 2024

View reviewed changes

girarda marked this pull request as ready for review May 9, 2024 02:52

girarda requested review from a team and brianjlai May 9, 2024 02:52

merge

db9721b

girarda added 2 commits May 9, 2024 14:21

fixes

28583e5

Revert "Airbyte CDK: use pytz.utc instead of datetime.utc (#38026)"

f75fb47

This reverts commit 5fe60b7.

Merge branch 'master' into alex/records_generator

d92ec4c

vercel bot deployed to Preview May 10, 2024 15:23 View deployment

girarda added 6 commits May 10, 2024 08:27

Revert "Revert "Airbyte CDK: use pytz.utc instead of datetime.utc (#3…

5ce3693

…8026)"" This reverts commit f75fb47.

typing

cbf9d12

Merge branch 'master' into alex/records_generator

b0456d1

Revert "Revert "Revert "Airbyte CDK: use pytz.utc instead of datetime…

43983ed

….utc (#38026)""" This reverts commit 5ce3693.

remove unused variable

cf0670c

hack. revert this before merging

c7b94f9

octavia-squidington-iii added area/connectors Connector related issues connectors/source/chargebee labels May 13, 2024

Revert "hack. revert this before merging"

6732c79

This reverts commit c7b94f9.

octavia-squidington-iii removed the area/connectors Connector related issues label May 13, 2024

Revert "Revert "Revert "Revert "Airbyte CDK: use pytz.utc instead of …

ff3a83e

…datetime.utc (#38026)"""" This reverts commit 43983ed.

brianjlai approved these changes May 13, 2024

View reviewed changes

merge master

9be956c

vercel bot deployed to Preview May 14, 2024 19:05 View deployment

girarda added 2 commits May 14, 2024 12:05

format

2cdcc4e

fix merge

69f5fb7

girarda merged commit fb11ca2 into master May 15, 2024
31 checks passed

girarda deleted the alex/records_generator branch May 15, 2024 01:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

low-code: Yield records from generators instead of keeping them in in-memory lists #36406

low-code: Yield records from generators instead of keeping them in in-memory lists #36406

girarda commented Mar 22, 2024 •

edited

Loading

vercel bot commented Mar 22, 2024 •

edited

Loading

girarda Mar 26, 2024

girarda Mar 26, 2024

artem1205 left a comment

girarda commented May 10, 2024

brianjlai left a comment

brianjlai May 13, 2024

brianjlai May 13, 2024

low-code: Yield records from generators instead of keeping them in in-memory lists #36406

low-code: Yield records from generators instead of keeping them in in-memory lists #36406

Conversation

girarda commented Mar 22, 2024 • edited Loading

What

How

Reading order

vercel bot commented Mar 22, 2024 • edited Loading

girarda Mar 26, 2024

Choose a reason for hiding this comment

girarda Mar 26, 2024

Choose a reason for hiding this comment

artem1205 left a comment

Choose a reason for hiding this comment

girarda commented May 10, 2024

brianjlai left a comment

Choose a reason for hiding this comment

brianjlai May 13, 2024

Choose a reason for hiding this comment

brianjlai May 13, 2024

Choose a reason for hiding this comment

girarda commented Mar 22, 2024 •

edited

Loading

vercel bot commented Mar 22, 2024 •

edited

Loading