Source Notion: Add Availability Strategy #30750

ChristoGrab · 2023-09-26T00:25:08Z

What

Resolve open tickets for implementing availability strategy and oncall 2712.

How

Inherit the base HttpAvailabilityStrategy to handle skipping streams when encountering a 403 error.
Added a slightly more Notion-specific message to output in the logs.
Small change to handle cases where no next_cursor exists when checking for a next_page_token, avoiding a KeyError. Added to this PR as it was also affecting the unit test for the 403 handling.

User Impact

Not a breaking change

Note

This PR will fail acceptance tests until schema changes have been merged. I'm submitting it now just to be ready.

vercel · 2023-09-26T00:25:13Z

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		Oct 9, 2023 1:09pm

github-actions · 2023-09-26T00:25:43Z

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.

This reverts commit bf2956c.

davydov-d

Minor comments regarding unit tests, otherwise looks good

davydov-d · 2023-09-27T06:25:14Z

airbyte-integrations/connectors/source-notion/unit_tests/test_streams.py

+    and returns custom Notion integration message.
+    """
+
+    with patch(f'source_notion.streams.{stream.__name__}._send_request') as mock_send_request:


I suggest you use requests_mock for cases like this. It is more readable and does not require patching magic methods. See example here

Gotcha, thanks for sharing the example. I've refactored the test as suggested 👍

Thanks! Let's ship it

davydov-d · 2023-09-27T06:26:13Z

airbyte-integrations/connectors/source-notion/unit_tests/test_streams.py

+        mock_resp.status_code = status_code
+
+        if status_code == 403:
+            mock_resp._content = b'{"object": "error", "status": 403, "code": "restricted_resource"}'


I think it's worth moving the response content to the parameters of the test along with the status code and others

….com/airbytehq/airbyte into christo/notion-availability-strategy

airbyte-oss-build-runner · 2023-09-27T16:02:21Z

source-notion test report (commit `5336f8bdc7`) - ❌

⏲️ Total pipeline duration: 04mn47s

Step	Result
Connector package install	✅
Build source-notion docker image for platform linux/x86_64	✅
Unit tests	✅
Acceptance tests	❌
Code format checks	✅
Validate airbyte-integrations/connectors/source-notion/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-notion test

airbyte-oss-build-runner · 2023-10-09T12:52:59Z

source-notion test report (commit `62ecaee3de`) - ❌

⏲️ Total pipeline duration: 04mn19s

Step	Result
Connector package install	✅
Build source-notion docker image for platform(s) linux/x86_64	✅
Unit tests	✅
Acceptance tests	✅
Code format checks	✅
Validate metadata for source-notion	✅
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-notion test

airbyte-oss-build-runner · 2023-10-09T13:16:44Z

source-notion test report (commit `2f310a96db`) - ✅

⏲️ Total pipeline duration: 04mn15s

Step	Result
Connector package install	✅
Build source-notion docker image for platform(s) linux/x86_64	✅
Unit tests	✅
Acceptance tests	✅
Code format checks	✅
Validate metadata for source-notion	✅
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-notion test

Co-authored-by: ChristoGrab <ChristoGrab@users.noreply.github.com>

ChristoGrab added 4 commits September 22, 2023 16:55

set availability strategy

03017a6

add unit test

1861677

clean up variables

bc6a825

add next_cursor fix and test

c0ddb0f

octavia-squidington-iii added area/connectors Connector related issues connectors/source/notion labels Sep 26, 2023

Automated Commit - Formatting Changes

bf2956c

Revert "Automated Commit - Formatting Changes"

3ff2c41

This reverts commit bf2956c.

ChristoGrab removed the connectors/source/mysql-strict-encrypt label Sep 26, 2023

vercel bot deployed to Preview September 26, 2023 00:50 View deployment

Merge branch 'master' into christo/notion-availability-strategy

7fc964a

ChristoGrab marked this pull request as ready for review September 26, 2023 01:08

vercel bot deployed to Preview September 26, 2023 01:09 View deployment

version change

60b6cb4

vercel bot deployed to Preview September 27, 2023 05:24 View deployment

Merge branch 'master' into christo/notion-availability-strategy

e89c410

vercel bot deployed to Preview September 27, 2023 05:27 View deployment

ChristoGrab and others added 2 commits September 26, 2023 23:43

format

f2ddc19

format

2b49e8d

davydov-d approved these changes Sep 27, 2023

View reviewed changes

ChristoGrab added 2 commits September 27, 2023 09:40

Merge branch 'christo/notion-availability-strategy' of https://github…

cc72146

….com/airbytehq/airbyte into christo/notion-availability-strategy

refactor unit_test

5336f8b

ChristoGrab mentioned this pull request Sep 28, 2023

Source Notion: Add suggested Streams and Optional Start Date #30780

Merged

Merge branch 'master' into christo/notion-availability-strategy

62ecaee

airbytehq deleted a comment from airbyte-oss-build-runner Oct 9, 2023

vercel bot deployed to Preview October 9, 2023 12:48 View deployment

airbytehq deleted a comment from airbyte-oss-build-runner Oct 9, 2023

ChristoGrab and others added 2 commits October 9, 2023 13:03

Automated Commit - Formatting Changes

8d5577a

last commit

2f310a9

ChristoGrab merged commit 537b304 into master Oct 9, 2023
24 checks passed

ChristoGrab deleted the christo/notion-availability-strategy branch October 9, 2023 13:31

edgao pushed a commit that referenced this pull request Oct 11, 2023

Source Notion: Add Availability Strategy (#30750)

7600644

Co-authored-by: ChristoGrab <ChristoGrab@users.noreply.github.com>

ariesgun pushed a commit to ariesgun/airbyte that referenced this pull request Oct 20, 2023

Source Notion: Add Availability Strategy (airbytehq#30750)

b9a5a30

Co-authored-by: ChristoGrab <ChristoGrab@users.noreply.github.com>

ariesgun pushed a commit to ariesgun/airbyte that referenced this pull request Oct 23, 2023

Source Notion: Add Availability Strategy (airbytehq#30750)

38b89e2

Co-authored-by: ChristoGrab <ChristoGrab@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Source Notion: Add Availability Strategy #30750

Source Notion: Add Availability Strategy #30750

ChristoGrab commented Sep 26, 2023 •

edited

vercel bot commented Sep 26, 2023 •

edited

github-actions bot commented Sep 26, 2023 •

edited by ChristoGrab

davydov-d left a comment

davydov-d Sep 27, 2023

ChristoGrab Sep 27, 2023

davydov-d Sep 28, 2023

davydov-d Sep 27, 2023

airbyte-oss-build-runner commented Sep 27, 2023

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

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

Source Notion: Add Availability Strategy #30750

Source Notion: Add Availability Strategy #30750

Conversation

ChristoGrab commented Sep 26, 2023 • edited

What

How

User Impact

Note

vercel bot commented Sep 26, 2023 • edited

github-actions bot commented Sep 26, 2023 • edited by ChristoGrab

Before Merging a Connector Pull Request

davydov-d left a comment

Choose a reason for hiding this comment

davydov-d Sep 27, 2023

Choose a reason for hiding this comment

ChristoGrab Sep 27, 2023

Choose a reason for hiding this comment

davydov-d Sep 28, 2023

Choose a reason for hiding this comment

davydov-d Sep 27, 2023

Choose a reason for hiding this comment

airbyte-oss-build-runner commented Sep 27, 2023

source-notion test report (commit 5336f8bdc7) - ❌

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

source-notion test report (commit 62ecaee3de) - ❌

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

source-notion test report (commit 2f310a96db) - ✅

ChristoGrab commented Sep 26, 2023 •

edited

vercel bot commented Sep 26, 2023 •

edited

github-actions bot commented Sep 26, 2023 •

edited by ChristoGrab

source-notion test report (commit `5336f8bdc7`) - ❌

source-notion test report (commit `62ecaee3de`) - ❌

source-notion test report (commit `2f310a96db`) - ✅