Alex/limit paginator #17197

girarda · 2022-09-27T02:17:08Z

What

Closes [low code cdk] Remove page_size from the LimitPaginator definition #16562 and Rename limit_option in LimitPaginator to match page_size #15575 by removing duplicate field from LimitPaginator
Renames LimitPaginator to DefaultPaginatorbecause it's not specific to limits
pin cdk version for greenhouse connector for safe release

Follow up item:

Update greenhouse

🚨 User Impact 🚨

Are there any breaking changes? What is the end result perceived by the user? If yes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

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.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub by running the /publish command described here
After the connector is published, connector added to connector index as described here
Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

Updating a connector

Community member or Airbyter

Grant edit access to maintainers (instructions)
Secrets in the connector's spec are annotated with airbyte_secret
Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
Code reviews completed
Documentation updated
- Connector's README.md
- Connector's bootstrap.md. See description and examples
- Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
PR name follows PR naming conventions

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.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub and connector version bumped by running the /publish command described here

Connector Generator

Issue acceptance criteria met
PR name follows PR naming conventions
If adding a new generator, add it to the list of scaffold modules being tested
The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
Documentation which references the generator is updated as needed

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

girarda · 2022-09-29T20:53:35Z

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

              inject_into: request_parameter
-              field_name: page_size
+              field_name: limit


renamed to limit as per comment at https://github.com/airbytehq/airbyte/pull/17197/files#diff-b36b8735005712a942bee18c2b6b9b824d9b08be821b16c4ea527d07e2f8901cR26

girarda · 2022-09-29T21:06:23Z

docs/connector-development/config-based/pagination.md


 Iterating over pages of result is different from iterating over stream slices.
 Stream slices have semantic value, for instance, a Datetime stream slice defines data for a specific date range. Two stream slices will have data for different date ranges.
 Conversely, pages don't have semantic value. More pages simply means that more records are to be read, without specifying any meaningful difference between the records of the first and later pages.

 The paginator is defined by

- `page_size`: The number of records to fetch in a single request
- `limit_option`: How to specify the page size in the outgoing HTTP request
+- `page_size_option`: How to specify the page size in the outgoing HTTP request


will add complete schema definitions in a follow up PR

girarda · 2022-09-29T21:24:23Z

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

        page_token_option (RequestOption): the request option to set the page token
        pagination_strategy (PaginationStrategy): Strategy defining how to get the next page token
        config (Config): connection config
        url_base (Union[InterpolatedString, str]): endpoint's base url
        decoder (Decoder): decoder to decode the response
    """

-    page_size: int


moved to the strategy

girarda · 2022-09-29T21:24:54Z

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

@@ -16,7 +16,7 @@
    author="Airbyte",
    author_email="contact@airbyte.io",
    packages=find_packages(),
-    install_requires=["airbyte-cdk", "dataclasses-jsonschema==2.15.1"],
+    install_requires=["airbyte-cdk==0.1.89", "dataclasses-jsonschema==2.15.1"],


freeze the version until greenhouse.yaml is updated

girarda · 2022-09-29T21:25:05Z

airbyte-integrations/connectors/source-sendgrid/source_sendgrid/sendgrid.yaml

@@ -16,7 +16,7 @@ definitions:
      type: "BearerAuthenticator"
      api_token: "{{ config.apikey }}"
  cursor_paginator:
-    type: LimitPaginator
+    type: DefaultPaginator


sendgrid is safe to update because it is not used in production

girarda · 2022-09-29T21:25:11Z

airbyte-integrations/connectors/source-sentry/source_sentry/sentry.yaml

@@ -18,7 +18,7 @@ definitions:
      type: "BearerAuthenticator"
      api_token: "{{ config.auth_token }}"
  paginator:
-    type: LimitPaginator
+    type: DefaultPaginator


sentry is safe to update because it is not used in production

brianjlai

i had a question related to why the cursor paginator needs to have a field for the page size. If anything we might want to leave it out of language til proven we needed it. But nothing to block on!

brianjlai · 2022-09-29T22:41:36Z

airbyte-cdk/python/CHANGELOG.md

 ## 0.1.89
+


Were these formatting changes intended? Did the linter/formatter get changed?

brianjlai · 2022-09-29T23:24:48Z

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

              inject_into: header
              field_name: page_size
            pagination_strategy:
              type: "OffsetIncrement"
+              page_size: 5
            page_token:


should this also be renamed to page_token_option? not the changed line, but the one below it (line 54)

brianjlai · 2022-09-30T00:01:16Z

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

@@ -30,6 +31,7 @@ class CursorPaginationStrategy(PaginationStrategy, JsonSchemaMixin):
    cursor_value: Union[InterpolatedString, str]
    config: Config
    options: InitVar[Mapping[str, Any]]
+    page_size: Optional[int] = None


What's the use case for page_size in a cursor paginator? We need the method to be defined to satisfy the interface, but maybe we should just return None? Also useful for making it easy to throw an error in the DefaultPaginator if the developer sets a page_size_option for a cursor paginator.

some APIs use a page size with a cursor (request 5 records per page and get the next page from the cursor)

ah okay, makes sense!

brianjlai · 2022-09-30T00:02:33Z

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

@@ -16,7 +16,7 @@
    author="Airbyte",
    author_email="contact@airbyte.io",
    packages=find_packages(),
-    install_requires=["airbyte-cdk", "dataclasses-jsonschema==2.15.1"],
+    install_requires=["airbyte-cdk==0.1.89", "dataclasses-jsonschema==2.15.1"],


girarda · 2022-09-30T00:57:58Z

/publish-cdk dry-run=true

🕑 https://github.com/airbytehq/airbyte/actions/runs/3155596188
✅ https://github.com/airbytehq/airbyte/actions/runs/3155596188

girarda · 2022-09-30T01:06:06Z

/publish-cdk dry-run=false

🕑 https://github.com/airbytehq/airbyte/actions/runs/3155627612
✅ https://github.com/airbytehq/airbyte/actions/runs/3155627612

* remove page size from limit paginator * Rename LimitPaginator to DefaultPaginator * rename limit_options * rename method * cleanup * comment * update schema file * Update changelog * pin greenhouse connector * fix doc in comment * fix changelog * missing - * bump * bump

girarda added 2 commits September 23, 2022 14:22

remove page size from limit paginator

1d23d06

Rename LimitPaginator to DefaultPaginator

8fa3cd9

github-actions bot added area/connectors Connector related issues area/documentation Improvements or additions to documentation CDK Connector Development Kit labels Sep 27, 2022

octavia-squidington-iii added connectors/source/sentry connectors/source/sendgrid labels Sep 27, 2022

girarda added 9 commits September 29, 2022 13:40

Merge branch 'master' into alex/limitPaginator

e815109

rename limit_options

11fddd2

rename method

f53f367

cleanup

2ea15be

comment

0066c2d

update schema file

9466611

Merge branch 'master' into alex/limitPaginator

9c73b8b

Update changelog

dfe187b

pin greenhouse connector

b66f338

octavia-squidington-iv added the connectors/source/greenhouse label Sep 29, 2022

girarda commented Sep 29, 2022

View reviewed changes

girarda requested a review from brianjlai September 29, 2022 21:25

girarda marked this pull request as ready for review September 29, 2022 21:25

girarda requested a review from a team as a code owner September 29, 2022 21:25

brianjlai approved these changes Sep 30, 2022

View reviewed changes

girarda added 6 commits September 29, 2022 17:38

fix doc in comment

af402ca

fix changelog

d8cc01b

missing -

8e681d5

bump

fa8d69d

merge master

61d8939

bump

acebc9e

girarda merged commit d6b79b8 into master Sep 30, 2022

girarda deleted the alex/limitPaginator branch September 30, 2022 02:24

octavia-squidington-iii mentioned this pull request Oct 5, 2022

Bump Airbyte version from 0.40.10 to 0.40.11 #17610

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Alex/limit paginator #17197

Alex/limit paginator #17197

girarda commented Sep 27, 2022 •

edited

girarda Sep 29, 2022

girarda Sep 29, 2022

girarda Sep 29, 2022

girarda Sep 29, 2022

brianjlai Sep 30, 2022

girarda Sep 29, 2022

girarda Sep 29, 2022

brianjlai left a comment

brianjlai Sep 29, 2022

brianjlai Sep 29, 2022 •

edited

brianjlai Sep 30, 2022

girarda Sep 30, 2022

brianjlai Sep 30, 2022

brianjlai Sep 30, 2022

girarda commented Sep 30, 2022 •

edited by github-actions bot

girarda commented Sep 30, 2022 •

edited by github-actions bot

Alex/limit paginator #17197

Alex/limit paginator #17197

Conversation

girarda commented Sep 27, 2022 • edited

What

Recommended reading order

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Tests

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

brianjlai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brianjlai Sep 29, 2022 • edited

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

girarda commented Sep 30, 2022 • edited by github-actions bot

girarda commented Sep 30, 2022 • edited by github-actions bot

girarda commented Sep 27, 2022 •

edited

brianjlai Sep 29, 2022 •

edited

girarda commented Sep 30, 2022 •

edited by github-actions bot

girarda commented Sep 30, 2022 •

edited by github-actions bot