✨Source Notion: add new stream `comments` #30324

ChristoGrab · 2023-09-11T21:20:52Z

What

Added stream for the Comments endpoint in Notion source connector.

How

Each stream slice returns a parent id for use in query params (following the pattern of Blocks stream).
To get incremental sync working properly, I used the parent page's last_edited_time as the cursor, adding it to the comment object as page_last_edited_time. I chose this approach because the order of the timestamps of the comments from one page to another can't be guaranteed, ie:

Page (Slice) I - Edited Time 2
    Comment A - Edited Time 1
    Comment B - Edited Time 4

Page (Slice) II - Edited Time 3
    Comment C - Edited Time 2
    Comment D - Edited Time 3

In this scenario, if we used the comment's last_edited_time as the cursor, the value of state will be updated to 4 after querying Page I, and in the following slice none of the comments in Page II will be synced. Because the pages query is sorted by LMD, by using the page's timestamp we can ensure that older comments in a page won't be skipped. Let me know if there's a better pattern to use that addresses this issue!

User Impact

No breaking changes

github-actions · 2023-09-11T21:21:17Z

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.

github-actions · 2023-09-12T14:10:12Z

source-notion test report (commit `b860a31c4e`) - ❌

⏲️ Total pipeline duration: 02mn27s

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

github-actions · 2023-09-12T20:03:25Z

source-notion test report (commit `ab631406ac`) - ❌

⏲️ Total pipeline duration: 02mn50s

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

sherifnada · 2023-09-12T20:02:59Z

airbyte-integrations/connectors/source-notion/source_notion/streams.py

+        return f"comments"
+
+    def request_params(self, next_page_token: Mapping[str, Any] = None, stream_slice: Mapping[str, Any] = None, **kwargs) -> MutableMapping[str, Any]:
+        block_id = stream_slice.get("block_id", "")


is there a valid situation where block_id wouldn't be set? or should this throw an error if it can't find block ID?

Good point, there's no valid reason that I can think of that block_id wouldn't be properly set, since every parent record's id is fetched directly from the API. I can remove the default argument (it will never trigger and after testing it would have just resulted in a 400 error anyway).

sherifnada · 2023-09-12T20:04:20Z

airbyte-integrations/connectors/source-notion/source_notion/streams.py

+
+        # Gather parent stream records in full
+        parent_records = self.parent.read_records(
+            sync_mode=SyncMode.full_refresh, cursor_field=self.parent.cursor_field, stream_state=stream_state


if we're syncing full refresh is there a reason to pass state?

Nope, that's my bad! I'll remove the argument

github-actions · 2023-09-13T21:34:49Z

source-notion test report (commit `bedd64ef84`) - ❌

⏲️ Total pipeline duration: 03mn14s

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

vercel · 2023-09-22T23:45:54Z

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 3:09pm

…m/airbytehq/airbyte into christo/notion-add-comment-stream

airbyte-oss-build-runner · 2023-10-09T15:06:34Z

source-notion test report (commit `cfff846f67`) - ❌

⏲️ Total pipeline duration: 05mn30s

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-09T15:16:29Z

source-notion test report (commit `3c9752af08`) - ✅

⏲️ Total pipeline duration: 04mn40s

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 17 commits September 5, 2023 15:22

add comments stream

0eb77d4

fix schema

ee4fd6e

remove bad error handling

b006eaf

add stream_slices for comments

d76bb90

comments full refresh works

f3f05d5

debugging state

2de91e8

fix stream_slice test

1143cca

update test case for read_records

84a738d

test blocks

2986453

incremental working

039c741

begin cleanup

3af8555

fix comments test cases

fd0b77a

refactor tests

815cc4c

change cursor in catalogs

0225a1d

fix

7d3baae

update version

dc29bb7

small fix

7958845

octavia-squidington-iii added area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/notion labels Sep 11, 2023

ChristoGrab added 2 commits September 11, 2023 17:25

add page_last_edited_time to schema

92b9234

changelog

3d707e9

ChristoGrab requested review from lazebnyi and sherifnada September 11, 2023 21:38

remove test comment

b860a31

ChristoGrab marked this pull request as ready for review September 12, 2023 14:05

fix format

ab63140

sherifnada reviewed Sep 12, 2023

View reviewed changes

remove extra args

a70b519

ChristoGrab marked this pull request as draft September 13, 2023 18:32

sherifnada approved these changes Sep 13, 2023

View reviewed changes

update expected_records

bedd64e

ChristoGrab marked this pull request as ready for review September 13, 2023 21:29

add block_id and missing enum values in schema

e418087

ChristoGrab marked this pull request as draft September 14, 2023 00:41

ChristoGrab added 2 commits September 13, 2023 20:44

format fix

d56dfcb

update capabilities section in docs

04d1ab6

vercel bot deployed to Preview September 22, 2023 23:50 View deployment

Automated Commit - Formatting Changes

70702b8

octavia-squidington-iii added the connectors/destination/pinecone label Sep 22, 2023

ChristoGrab and others added 4 commits October 5, 2023 16:08

update docs page

befaa16

Merge branch 'christo/notion-add-comment-stream' of https://github.co…

81ac2d3

…m/airbytehq/airbyte into christo/notion-add-comment-stream

Merge branch 'master' into christo/notion-add-comment-stream

417399b

Automated Commit - Formatting Changes

cfff846

ChristoGrab marked this pull request as ready for review October 9, 2023 14:58

delete inapp doc

05584a6

remove fstring

3c9752a

ChristoGrab merged commit 3b0269f into master Oct 9, 2023
24 checks passed

ChristoGrab deleted the christo/notion-add-comment-stream branch October 9, 2023 15:40

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

✨Source Notion: add new stream comments (#30324)

91c6201

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 new stream comments (airbytehq#30324)

f80aa39

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 new stream comments (airbytehq#30324)

175a91b

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 new stream `comments` #30324

✨Source Notion: add new stream `comments` #30324

ChristoGrab commented Sep 11, 2023 •

edited

github-actions bot commented Sep 11, 2023 •

edited by ChristoGrab

github-actions bot commented Sep 12, 2023

github-actions bot commented Sep 12, 2023

sherifnada Sep 12, 2023

ChristoGrab Sep 12, 2023

sherifnada Sep 12, 2023

ChristoGrab Sep 12, 2023

github-actions bot commented Sep 13, 2023

vercel bot commented Sep 22, 2023 •

edited

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

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

✨Source Notion: add new stream comments #30324

✨Source Notion: add new stream comments #30324

Conversation

ChristoGrab commented Sep 11, 2023 • edited

What

How

Recommended reading order

User Impact

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

Before Merging a Connector Pull Request

github-actions bot commented Sep 12, 2023

source-notion test report (commit b860a31c4e) - ❌

github-actions bot commented Sep 12, 2023

source-notion test report (commit ab631406ac) - ❌

sherifnada Sep 12, 2023

Choose a reason for hiding this comment

ChristoGrab Sep 12, 2023

Choose a reason for hiding this comment

sherifnada Sep 12, 2023

Choose a reason for hiding this comment

ChristoGrab Sep 12, 2023

Choose a reason for hiding this comment

github-actions bot commented Sep 13, 2023

source-notion test report (commit bedd64ef84) - ❌

vercel bot commented Sep 22, 2023 • edited

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

source-notion test report (commit cfff846f67) - ❌

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

source-notion test report (commit 3c9752af08) - ✅

✨Source Notion: add new stream `comments` #30324

✨Source Notion: add new stream `comments` #30324

ChristoGrab commented Sep 11, 2023 •

edited

github-actions bot commented Sep 11, 2023 •

edited by ChristoGrab

source-notion test report (commit `b860a31c4e`) - ❌

source-notion test report (commit `ab631406ac`) - ❌

source-notion test report (commit `bedd64ef84`) - ❌

vercel bot commented Sep 22, 2023 •

edited

source-notion test report (commit `cfff846f67`) - ❌

source-notion test report (commit `3c9752af08`) - ✅