Source notion: fix schema #20639

davydov-d · 2022-12-19T13:40:51Z

What

https://github.com/airbytehq/oncall/issues/1047
#19777

How

Fix stream schema
Set test strictness level to high

davydov-d · 2022-12-19T13:41:59Z

/test connector=connectors/source-notion

🕑 connectors/source-notion https://github.com/airbytehq/airbyte/actions/runs/3731980197
✅ connectors/source-notion https://github.com/airbytehq/airbyte/actions/runs/3731980197
Python tests coverage:

Name                        Stmts   Miss  Cover
-----------------------------------------------
source_notion/__init__.py       2      0   100%
source_notion/streams.py      125      5    96%
source_notion/source.py        37      5    86%
source_notion/utils.py          9      4    56%
-----------------------------------------------
TOTAL                         173     14    92%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       140      5    96%   87, 93, 238, 242-243
	 source_acceptance_test/conftest.py                     208     92    56%   36, 42-44, 49, 54, 77, 83, 89-91, 110, 115-117, 123-125, 131-132, 137-138, 143, 149, 158-167, 173-178, 193, 217, 248, 254, 262-267, 275-280, 288-301, 306-312, 319-330, 337-353
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              402    115    71%   53, 58, 93-104, 109-116, 120-121, 125-126, 308, 346-363, 376-387, 391-396, 402, 435-440, 478-485, 528-530, 533, 598-606, 618-621, 626, 682-683, 689, 692, 728-738, 751-776
	 source_acceptance_test/tests/test_incremental.py       158     14    91%   52-59, 64-77, 240
	 source_acceptance_test/utils/asserts.py                 39      2    95%   62-63
	 source_acceptance_test/utils/common.py                  94     10    89%   16-17, 32-38, 72, 75
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       133     33    75%   24-27, 46-47, 50-54, 57-58, 73-75, 78-80, 83-85, 88-90, 93-95, 124-125, 159-161, 208
	 source_acceptance_test/utils/json_schema_helper.py     107     13    88%   30-31, 38, 41, 65-68, 96, 120, 192-194
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1603    336    79%

Build Passed

Test summary info:

All Passed

evantahler

The changes look good, and I've got high-confidence with test_strictness_level: high. Please add some more information into the change log as to what the breaking change is for users of the connector.

airbyte-integrations/connectors/source-notion/acceptance-test-config.yml

evantahler · 2022-12-19T19:26:56Z

docs/integrations/sources/notion.md

@@ -83,7 +83,8 @@ The connector is restricted by Notion [request limits](https://developers.notion
 ## Changelog

 | Version | Date       | Pull Request                                             | Subject                                                         |
-| :------ | :--------- | :------------------------------------------------------- | :-------------------------------------------------------------- |
+|:--------|:-----------|:---------------------------------------------------------|:----------------------------------------------------------------|
+| 1.0.0   | 2022-12-19 | [20639](https://github.com/airbytehq/airbyte/pull/20639) | Fix `Pages` stream schema                                       |


You noted that this was a breaking change (1.0) - what should users expect to see that's changed from previous versions in their records?

No changes in records - the schema itself is changed, meaning the discover should yield different catalog now. Btw, I'm pretty sure this should cause a backward compatibility test to fail but it does not. May I ask you to take a look at it? I believe this is because there was no type field in the root of this object

@davydov-d I think your intuition is correct. Even if SAT did not detect the backward incompatibility, this is a breaking change on the page stream. Could you please sync with @girarda and @jnr0790 to assess the impact of this breaking change and check if customer outreach is needed?
I'm under the impression you're fixing a nonworking stream, but it'd be worth double-checking that the stream is failing for all the users. It would help to spot the differences if you could share the output of discover in 0.1.10 vs 1.0.0.

Sure! I'm attaching the output for the Pages stream as files as it's pretty big

0.1.10.txt
1.0.0.txt

The change you introduced is removing and adding properties.
The backward compatibility checks pass because:

Removal of properties is considered backward compatible because downstream normalization is not destructive: the previously created column for this property won't be deleted

The addition of properties will not break an existing catalog but will trigger schema validation errors that are not failing syncs.

@evantahler @sherifnada wdyt? Shall we make the backward compatibility test fail in this situation?

I think that the change you introduced are not functionally breaking changes but will lead to very messy destination schema after normalization.
I'd suggest you locally try a sync with 0.1.10 and then upgrade to 1.0.0:

check if the sync succeeds

check how the destination schema looks like

I tracked this evolution in a issue: #20754

just to clarify - any action item here to be done? does it still make sense to run a sync locally to look at the destination schema regarding we have decided to treat it as an incompatible change or may I resolve this thread?

This will be affecting all cloud users with a Notion connection correct?

@jnr0790 this will affect those users with Pages stream enabled. I will post a list of them in the issue

@davydov-d can you tag me and Erica on that comment cc @erica-airbyte

davydov-d · 2022-12-20T09:05:49Z

/test connector=connectors/source-notion

🕑 connectors/source-notion https://github.com/airbytehq/airbyte/actions/runs/3739030417
✅ connectors/source-notion https://github.com/airbytehq/airbyte/actions/runs/3739030417
Python tests coverage:

Name                        Stmts   Miss  Cover
-----------------------------------------------
source_notion/__init__.py       2      0   100%
source_notion/streams.py      125      5    96%
source_notion/source.py        37      5    86%
source_notion/utils.py          9      4    56%
-----------------------------------------------
TOTAL                         173     14    92%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       140      5    96%   87, 93, 238, 242-243
	 source_acceptance_test/conftest.py                     208     92    56%   36, 42-44, 49, 54, 77, 83, 89-91, 110, 115-117, 123-125, 131-132, 137-138, 143, 149, 158-167, 173-178, 193, 217, 248, 254, 262-267, 275-280, 288-301, 306-312, 319-330, 337-353
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              402    115    71%   53, 58, 93-104, 109-116, 120-121, 125-126, 308, 346-363, 376-387, 391-396, 402, 435-440, 478-485, 528-530, 533, 598-606, 618-621, 626, 682-683, 689, 692, 728-738, 751-776
	 source_acceptance_test/tests/test_incremental.py       158     14    91%   52-59, 64-77, 240
	 source_acceptance_test/utils/asserts.py                 39      2    95%   62-63
	 source_acceptance_test/utils/common.py                  94     10    89%   16-17, 32-38, 72, 75
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       133     33    75%   24-27, 46-47, 50-54, 57-58, 73-75, 78-80, 83-85, 88-90, 93-95, 124-125, 159-161, 208
	 source_acceptance_test/utils/json_schema_helper.py     107     13    88%   30-31, 38, 41, 65-68, 96, 120, 192-194
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1603    336    79%

Build Passed

Test summary info:

All Passed

airbyte-integrations/connectors/source-notion/integration_tests/expected_records.txt

ryankfu · 2023-01-09T21:26:20Z

@davydov-d Are there still any blockers for this PR? We recently saw this issue popped up today within our on-call and are curious if you can give an update since the code freeze has been lifted

cc: @etsybaev

…hema

davydov-d · 2023-01-10T12:29:32Z

/test connector=connectors/source-notion

🕑 connectors/source-notion https://github.com/airbytehq/airbyte/actions/runs/3883334444
❌ connectors/source-notion https://github.com/airbytehq/airbyte/actions/runs/3883334444
🐛 https://gradle.com/s/6vwmpnnq47sco

Build Failed

Test summary info:

=========================== short test summary info ============================
ERROR test_core.py::TestBasicRead::test_read[inputs0] - FileNotFoundError: [E...
==================== 31 passed, 1 error in 77.84s (0:01:17) ====================

davydov-d · 2023-01-10T12:31:10Z

@ryankfu sorry for the long delay - i have been on a sick leave. I'll sync with the TCS and deploy this change asap

…hema

davydov-d · 2023-01-10T13:09:15Z

/test connector=connectors/source-notion

🕑 connectors/source-notion https://github.com/airbytehq/airbyte/actions/runs/3883642548
❌ connectors/source-notion https://github.com/airbytehq/airbyte/actions/runs/3883642548
🐛 https://gradle.com/s/govod5x2uylho

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestBasicRead::test_read[inputs0] - Failed: Stream pages...
=================== 1 failed, 31 passed in 93.68s (0:01:33) ====================

davydov-d · 2023-01-10T14:23:12Z

/test connector=connectors/source-notion

🕑 connectors/source-notion https://github.com/airbytehq/airbyte/actions/runs/3884231314
✅ connectors/source-notion https://github.com/airbytehq/airbyte/actions/runs/3884231314
Python tests coverage:

Name                        Stmts   Miss  Cover
-----------------------------------------------
source_notion/__init__.py       2      0   100%
source_notion/streams.py      125      5    96%
source_notion/source.py        37      5    86%
source_notion/utils.py          9      4    56%
-----------------------------------------------
TOTAL                         173     14    92%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       141      5    96%   87, 93, 239, 243-244
	 source_acceptance_test/conftest.py                     211     95    55%   36, 42-44, 49, 54, 77, 83, 89-91, 110, 115-117, 123-125, 131-132, 137-138, 143, 149, 158-167, 173-178, 193, 217, 248, 254, 262-267, 275-285, 293-306, 311-317, 324-335, 342-358
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              402    115    71%   53, 58, 93-104, 109-116, 120-121, 125-126, 308, 346-363, 376-387, 391-396, 402, 435-440, 478-485, 528-530, 533, 598-606, 618-621, 626, 682-683, 689, 692, 728-738, 751-776
	 source_acceptance_test/tests/test_incremental.py       160     14    91%   56-63, 68-81, 244
	 source_acceptance_test/utils/asserts.py                 39      2    95%   62-63
	 source_acceptance_test/utils/common.py                  94     10    89%   16-17, 32-38, 72, 75
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       133     33    75%   24-27, 46-47, 50-54, 57-58, 73-75, 78-80, 83-85, 88-90, 93-95, 124-125, 159-161, 208
	 source_acceptance_test/utils/json_schema_helper.py     107     13    88%   30-31, 38, 41, 65-68, 96, 120, 192-194
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1609    339    79%

Build Passed

Test summary info:

All Passed

erica-airbyte · 2023-01-10T16:15:34Z

@davydov-d this is not been merged yet correct? I think we are waiting on a list of customers that this will affect that we need to reach out to before the change go to production.

davydov-d · 2023-01-10T16:28:22Z

@erica-airbyte that's right, it is not merged yet. The list is here as Juan asked (I will take a look if something changed for the last three weeks and let you know in the issue)

sentry-io · 2023-01-10T22:42:25Z

Sentry issue: CONNECTOR-INCIDENT-MANAGEMENT-SN

davydov-d · 2023-01-11T09:56:37Z

/publish connector=connectors/source-notion

🕑 Publishing the following connectors:
connectors/source-notion
https://github.com/airbytehq/airbyte/actions/runs/3891650784

Connector	Did it publish?	Were definitions generated?
connectors/source-notion	✅	✅

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

…hema

* #1047 oncall - Source Notion: fix Pages stream schema * #1047 oncall - Source Notion: fix Pages stream schema * #1047 oncall - Source Notion: upd changelog * #1047 source notion - remove ignored fields * #1047 source notion: fix SATs * #1047 source notion: upd expected records * auto-bump connector version Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>

davydov-d added 2 commits December 19, 2022 15:39

#1047 oncall - Source Notion: fix Pages stream schema

c349b13

#1047 oncall - Source Notion: fix Pages stream schema

be4a767

octavia-squidington-iv added area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/notion labels Dec 19, 2022

#1047 oncall - Source Notion: upd changelog

78fc89d

davydov-d requested review from lazebnyi, roman-yermilov-gl and a team December 19, 2022 13:42

davydov-d linked an issue Dec 19, 2022 that may be closed by this pull request

Stream pages in Notion source has incorrect schema for field properties #19777

Closed

evantahler requested changes Dec 19, 2022

View reviewed changes

#1047 source notion - remove ignored fields

fb1c52d

davydov-d requested a review from evantahler December 20, 2022 11:41

evantahler approved these changes Dec 21, 2022

View reviewed changes

airbyte-integrations/connectors/source-notion/integration_tests/expected_records.txt Outdated Show resolved Hide resolved

lazebnyi approved these changes Jan 3, 2023

View reviewed changes

Merge branch 'master' into ddavydov/#1047-oncall-source-notion-fix-sc…

e78514d

…hema

davydov-d added 2 commits January 10, 2023 15:08

#1047 source notion: fix SATs

d097f09

Merge branch 'master' into ddavydov/#1047-oncall-source-notion-fix-sc…

739eb91

…hema

#1047 source notion: upd expected records

5c7aff2

auto-bump connector version

f43810e

octavia-squidington-iii temporarily deployed to more-secrets January 11, 2023 10:22 — with GitHub Actions Inactive

octavia-squidington-iii temporarily deployed to more-secrets January 11, 2023 10:23 — with GitHub Actions Inactive

octavia-squidington-iii temporarily deployed to more-secrets January 11, 2023 10:56 — with GitHub Actions Inactive

Merge branch 'master' into ddavydov/#1047-oncall-source-notion-fix-sc…

e6bc273

…hema

davydov-d temporarily deployed to more-secrets January 11, 2023 11:12 — with GitHub Actions Inactive

davydov-d temporarily deployed to more-secrets January 11, 2023 11:13 — with GitHub Actions Inactive

davydov-d temporarily deployed to more-secrets January 11, 2023 11:56 — with GitHub Actions Inactive

Merge branch 'master' into ddavydov/#1047-oncall-source-notion-fix-sc…

d063618

…hema

davydov-d temporarily deployed to more-secrets January 11, 2023 14:12 — with GitHub Actions Inactive

davydov-d merged commit 9d4dd48 into master Jan 11, 2023

davydov-d deleted the ddavydov/#1047-oncall-source-notion-fix-schema branch January 11, 2023 14:57

philippeboyd mentioned this pull request Jan 11, 2023

Source Notion: When syncing, all streams are empty except Users #16987

Closed

octavia-squidington-iii mentioned this pull request Jan 12, 2023

Bump Airbyte version from 0.40.27 to 0.40.28 #21359

Merged

This was referenced Jan 16, 2023

Source Notion: enable high test strictness level in SAT #19057

Closed

Source Notion: enable high test strictness level in SAT #19170

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Source notion: fix schema #20639

Source notion: fix schema #20639

davydov-d commented Dec 19, 2022

davydov-d commented Dec 19, 2022 •

edited by github-actions bot

Loading

evantahler left a comment

evantahler Dec 19, 2022

davydov-d Dec 20, 2022

alafanechere Dec 20, 2022

davydov-d Dec 20, 2022

alafanechere Dec 20, 2022

alafanechere Dec 21, 2022

davydov-d Dec 21, 2022

jnr0790 Dec 22, 2022

davydov-d Dec 23, 2022

jnr0790 Dec 23, 2022

davydov-d commented Dec 20, 2022 •

edited by github-actions bot

Loading

ryankfu commented Jan 9, 2023 •

edited

Loading

davydov-d commented Jan 10, 2023 •

edited by github-actions bot

Loading

davydov-d commented Jan 10, 2023

davydov-d commented Jan 10, 2023 •

edited by github-actions bot

Loading

davydov-d commented Jan 10, 2023 •

edited by github-actions bot

Loading

erica-airbyte commented Jan 10, 2023

davydov-d commented Jan 10, 2023

sentry-io bot commented Jan 10, 2023

davydov-d commented Jan 11, 2023 •

edited by github-actions bot

Loading

Source notion: fix schema #20639

Source notion: fix schema #20639

Conversation

davydov-d commented Dec 19, 2022

What

How

davydov-d commented Dec 19, 2022 • edited by github-actions bot Loading

Build Passed

evantahler left a 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

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

davydov-d commented Dec 20, 2022 • edited by github-actions bot Loading

Build Passed

ryankfu commented Jan 9, 2023 • edited Loading

davydov-d commented Jan 10, 2023 • edited by github-actions bot Loading

Build Failed

davydov-d commented Jan 10, 2023

davydov-d commented Jan 10, 2023 • edited by github-actions bot Loading

Build Failed

davydov-d commented Jan 10, 2023 • edited by github-actions bot Loading

Build Passed

erica-airbyte commented Jan 10, 2023

davydov-d commented Jan 10, 2023

sentry-io bot commented Jan 10, 2023

davydov-d commented Jan 11, 2023 • edited by github-actions bot Loading

davydov-d commented Dec 19, 2022 •

edited by github-actions bot

Loading

davydov-d commented Dec 20, 2022 •

edited by github-actions bot

Loading

ryankfu commented Jan 9, 2023 •

edited

Loading

davydov-d commented Jan 10, 2023 •

edited by github-actions bot

Loading

davydov-d commented Jan 10, 2023 •

edited by github-actions bot

Loading

davydov-d commented Jan 10, 2023 •

edited by github-actions bot

Loading

davydov-d commented Jan 11, 2023 •

edited by github-actions bot

Loading