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.

Sign up for GitHub

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 Monday: fix boards stream 25 row limit in source-monday #25277

Merged
merged 6 commits into from
Apr 27, 2023
Merged

Source Monday: fix boards stream 25 row limit in source-monday #25277

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c450ad2
fix boards stream 25 row limit in source-monday
igsaf2 Apr 18, 2023
054258b
Merge branch 'master' into fix-monday-boards
igsaf2 Apr 26, 2023
c0edd68
update docs and dockerfile
marcosmarxm Apr 26, 2023
d401b10
auto-bump connector version
octavia-squidington-iii Apr 26, 2023
0a3e77a
Merge branch 'master' into fix-monday-boards
marcosmarxm Apr 26, 2023
825ea40
Merge branch 'master' into fix-monday-boards
marcosmarxm Apr 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion airbyte-config-oss/init-oss/src/main/resources/seed/source_definitions.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1272,7 +1272,7 @@
- name: Monday
sourceDefinitionId: 80a54ea2-9959-4040-aac1-eee42423ec9b
dockerRepository: airbyte/source-monday
dockerImageTag: 0.2.3
dockerImageTag: 0.2.4
documentationUrl: https://docs.airbyte.com/integrations/sources/monday
icon: monday.svg
sourceType: api
Expand Down
2 changes: 1 addition & 1 deletion airbyte-integrations/connectors/source-monday/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,5 @@ COPY source_monday ./source_monday
ENV AIRBYTE_ENTRYPOINT "python /airbyte/integration_code/main.py"
ENTRYPOINT ["python", "/airbyte/integration_code/main.py"]

LABEL io.airbyte.version=0.2.3
LABEL io.airbyte.version=0.2.4
LABEL io.airbyte.name=airbyte/source-monday
1 change: 1 addition & 0 deletions airbyte-integrations/connectors/source-monday/source_monday/manifest.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ definitions:
$parameters:
name: "boards"
path: ""
items_per_page: 100
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current Pagination already uses 100 items check default_paginator component. Are you able to build the dev version and test it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have run python main.py read --config secrets/config.json --catalog configured_stream.json . It works for us with 200+ boards.
This is not only about pagination, but also about limit parameter;

airbyte/airbyte-integrations/connectors/source-monday/source_monday/manifest.yaml

Line 22 in 44fa284

limit: "{{ parameters['items_per_page'] }}"

Right now the logic is following:

  1. limit becomes None in here
  2. Because limit is not specified, Monday returns default 25 rows for boards.
  3. Pagination checks if it needs to visit another page with the condition: len(last_records) < self.page_size. And it gets 25 < 100. So there is no query for the second page.

This PR solves the issue: the limit is set and pagination starts working when needed. Although it does not look self-explaining

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

(venv) igsaf@igsaf-nix:~/IdeaProjects/airbyte/airbyte-integrations/connectors/source-monday (fix-monday-boards) $ python main.py read --config secrets/config.json --catalog configured_stream.json | tail -n 4 
{"type": "LOG", "log": {"level": "INFO", "message": "Read 221 records from boards stream"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Finished syncing boards"}}
{"type": "LOG", "log": {"level": "INFO", "message": "SourceMonday runtimes:\nSyncing stream boards 0:00:21.135193"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Finished syncing SourceMonday"}}

teams_stream:
$ref: "#/definitions/base_nopagination_stream"
$parameters:
Expand Down
1 change: 1 addition & 0 deletions docs/integrations/sources/monday.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ The Monday connector should not run into Monday API limitations under normal usa

| Version | Date | Pull Request | Subject |
|:--------|:-----------|:---------------------------------------------------------|:------------------------------------------------|
| 0.2.4 | 2023-04-26 | [25277](https://github.com/airbytehq/airbyte/pull/25277) | Increase row limit to 100 |
| 0.2.3 | 2023-03-06 | [23231](https://github.com/airbytehq/airbyte/pull/23231) | Publish using low-code CDK Beta version |
| 0.2.2 | 2023-01-04 | [20996](https://github.com/airbytehq/airbyte/pull/20996) | Fix json schema loader |
| 0.2.1 | 2022-12-15 | [20533](https://github.com/airbytehq/airbyte/pull/20533) | Bump CDK version |
Expand Down