Add SAT test for no duplicates in `enum` properties #19243

erohmensing · 2022-11-09T17:44:44Z

What

Stop users from writing enum lists that contain duplicate values, since we don't want duplicates values in the UI list.
Close [SAT] Test Spec: Check for duplicate values under enum #18740

How

Decide to enforce this, although some jsonschema drafts, in particular draft7 doesn't canonically (see thread for the rabbit hole that lead to this conclusion)
Add SAT test that checks correct usage of enum by Airbyte's standards

🚨 User Impact 🚨

Users with custom connectors that contain specsc with duplicate enum values will no longer pass SAT. This shouldn't affect anything in production, but will stop connectors from being published until they pass the new test.

Pre-merge Checklist

Run /test on a few connectors as sanity checks
- connector without enum: source-airtable
- source with enum: source-amazon-ads

github-actions · 2022-11-09T17:45:55Z

Affected Connector Report

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to do the following as needed:

Run integration tests
Bump connector version
Add changelog
Publish the new version

❌ Sources (63)

Connector	Version	Changelog	Publish
`source-airtable`	`0.1.3`	✅	✅
`source-amazon-ads`	`0.1.25`	✅	✅
`source-amazon-seller-partner`	`0.2.28`	✅	✅
`source-amazon-sqs`	`0.1.0`	✅	✅
`source-amplitude`	`0.1.17`	✅	✅
`source-appsflyer`	`0.1.0`	⚠ (doc not found)	⚠ (not in seed)
`source-asana`	`0.1.4`	✅	✅
`source-azure-table`	`0.1.3`	✅	✅
`source-braintree`	`0.1.3`	✅	✅
`source-cart`	`0.2.0`	❌ (changelog missing)	✅
`source-chargebee`	`0.1.16`	✅	❌ (diff seed version)
`source-commercetools`	`0.1.0`	✅	✅
`source-confluence`	`0.1.1`	✅	✅
`source-datadog`	`0.1.0`	✅	✅
`source-delighted`	`0.1.4`	✅	✅
`source-drift`	`0.2.5`	✅	✅
`source-facebook-marketing`	`0.2.72`	✅	✅
`source-facebook-pages`	`0.1.6`	✅	✅
`source-freshcaller`	`0.1.0`	✅	✅
`source-freshsales`	`0.1.2`	✅	✅
`source-freshservice`	`0.1.1`	✅	✅
`source-github`	`0.3.7`	✅	✅
`source-gitlab`	`0.1.6`	✅	✅
`source-google-ads`	`0.2.3`	✅	✅
`source-google-search-console`	`0.1.18`	✅	✅
`source-greenhouse`	`0.3.0`	✅	✅
`source-harvest`	`0.1.11`	✅	✅
`source-instagram`	`1.0.0`	✅	✅
`source-iterable`	`0.1.21`	✅	✅
`source-klaviyo`	`0.1.10`	✅	✅
`source-lemlist`	`0.1.1`	❌ (changelog missing)	❌ (diff seed version)
`source-lever-hiring`	`0.1.3`	✅	✅
`source-linnworks`	`0.1.5`	❌ (changelog missing)	✅
`source-mailchimp`	`0.2.15`	✅	✅
`source-mailgun`	`0.1.0`	⚠ (doc not found)	✅
`source-monday`	`0.1.4`	✅	✅
`source-notion`	`0.1.10`	✅	✅
`source-okta`	`0.1.13`	✅	✅
`source-onesignal`	`0.1.2`	✅	✅
`source-openweather`	`0.1.6`	✅	✅
`source-outreach`	`0.1.2`	✅	✅
`source-pardot`	`0.1.0`	⚠ (doc not found)	⚠ (not in seed)
`source-paystack`	`0.1.1`	✅	✅
`source-pinterest`	`0.1.9`	✅	✅
`source-pipedrive`	`0.1.13`	✅	✅
`source-plaid`	`0.3.2`	❌ (changelog missing)	✅
`source-posthog`	`0.1.7`	✅	✅
`source-prestashop`	`0.2.0`	⚠ (doc not found)	✅
`source-quickbooks-singer`	`0.1.5`	⚠ (doc not found)	✅
`source-recharge`	`0.2.4`	❌ (changelog missing)	✅
`source-retently`	`0.1.2`	✅	✅
`source-salesforce`	`1.0.24`	✅	✅
`source-salesloft`	`0.1.3`	✅	✅
`source-sendgrid`	`0.2.16`	✅	✅
`source-sentry`	`0.1.7`	✅	✅
`source-strava`	`0.1.2`	✅	✅
`source-surveymonkey`	`0.1.11`	✅	✅
`source-tplcentral`	`0.1.1`	✅	⚠ (not in seed)
`source-twilio`	`0.1.13`	✅	✅
`source-youtube-analytics`	`0.1.3`	✅	✅
`source-zendesk-sunshine`	`0.1.1`	✅	✅
`source-zendesk-talk`	`0.1.5`	✅	✅
`source-zenloop`	`0.1.3`	✅	✅

See "Actionable Items" below for how to resolve warnings and errors.

✅ Destinations (0)

Connector	Version	Changelog	Publish

See "Actionable Items" below for how to resolve warnings and errors.

Actionable Items

(click to expand)

Category	Status	Actionable Item
Version	❌ mismatch	The version of the connector is different from its normal variant. Please bump the version of the connector.
	⚠ doc not found	The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like `source-jdbc` is not published or documented). Please double-check to make sure that it is not a bug.
Changelog	⚠ doc not found	The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like `source-jdbc` is not published or documented). Please double-check to make sure that it is not a bug.
	❌ changelog missing	There is no chnagelog for the current version of the connector. If you are the author of the current version, please add a changelog.
Publish	⚠ not in seed	The connector is not in the seed file (e.g. `source_definitions.yaml`), so its publication status cannot be checked. This can be normal (e.g. some connectors are cloud-specific, and only listed in the cloud seed file). Please double-check to make sure that it is not a bug.
	❌ diff seed version	The connector exists in the seed file, but the latest version is not listed there. This usually means that the latest version is not published. Please use the `/publish` command to publish the latest version.

erohmensing · 2022-11-09T18:50:06Z

/test connector=connectors/source-airtable

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

Name                          Stmts   Miss  Cover
-------------------------------------------------
source_airtable/__init__.py       2      0   100%
source_airtable/helpers.py       36      2    94%
source_airtable/source.py        74     22    70%
-------------------------------------------------
TOTAL                           112     24    79%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       133      3    98%   87, 93, 230
	 source_acceptance_test/conftest.py                     196     97    51%   35, 41-43, 48, 54, 60, 66, 72-74, 80-95, 100, 105-107, 113-115, 121-122, 127-128, 133, 139, 148-157, 163-168, 232, 238, 244-250, 258-263, 271-284, 289-295, 302-313, 320-336
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              337    106    69%   39, 50-58, 63-70, 74-75, 79-80, 178, 216-233, 242-250, 254-259, 265, 298-303, 341-348, 391-393, 396, 461-469, 498-499, 505, 508, 544-554, 567-592
	 source_acceptance_test/tests/test_incremental.py       145     20    86%   21-23, 29-31, 36-43, 48-61, 224
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  77     10    87%   15-16, 24-30, 64, 67
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/config_migration.py        23     23     0%   5-37
	 source_acceptance_test/utils/connector_runner.py       112     50    55%   23-26, 32, 36, 39-68, 71-73, 76-78, 81-83, 86-88, 91-93, 96-114, 148-150
	 source_acceptance_test/utils/json_schema_helper.py     105     13    88%   30-31, 38, 41, 65-68, 96, 120, 190-192
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1487    376    75%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: not found in the config.
================= 27 passed, 1 skipped, 28 warnings in 31.18s ==================

erohmensing · 2022-11-09T18:50:20Z

/test connector=connectors/source-amazon-ads

🕑 connectors/source-amazon-ads https://github.com/airbytehq/airbyte/actions/runs/3430958478
✅ connectors/source-amazon-ads https://github.com/airbytehq/airbyte/actions/runs/3430958478
Python tests coverage:

Name                                                              Stmts   Miss  Cover
-------------------------------------------------------------------------------------
source_amazon_ads/utils.py                                            9      0   100%
source_amazon_ads/streams/sponsored_products.py                      41      0   100%
source_amazon_ads/streams/sponsored_display.py                       31      0   100%
source_amazon_ads/streams/sponsored_brands.py                        26      0   100%
source_amazon_ads/streams/report_streams/products_report.py          18      0   100%
source_amazon_ads/streams/report_streams/display_report.py           16      0   100%
source_amazon_ads/streams/report_streams/brands_video_report.py      10      0   100%
source_amazon_ads/streams/report_streams/brands_report.py            10      0   100%
source_amazon_ads/streams/report_streams/__init__.py                  5      0   100%
source_amazon_ads/streams/profiles.py                                21      0   100%
source_amazon_ads/streams/attribution_report.py                      81      0   100%
source_amazon_ads/streams/__init__.py                                 7      0   100%
source_amazon_ads/schemas/sponsored_products.py                      37      0   100%
source_amazon_ads/schemas/sponsored_display.py                       31      0   100%
source_amazon_ads/schemas/sponsored_brands.py                        22      0   100%
source_amazon_ads/schemas/profile.py                                 16      0   100%
source_amazon_ads/schemas/attribution_report.py                      21      0   100%
source_amazon_ads/schemas/__init__.py                                 7      0   100%
source_amazon_ads/constants.py                                        1      0   100%
source_amazon_ads/__init__.py                                         2      0   100%
source_amazon_ads/streams/common.py                                  79      1    99%
source_amazon_ads/schemas/common.py                                  51      1    98%
source_amazon_ads/source.py                                          44      1    98%
source_amazon_ads/streams/report_streams/report_streams.py          240     19    92%
-------------------------------------------------------------------------------------
TOTAL                                                               826     22    97%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       133      3    98%   87, 93, 230
	 source_acceptance_test/conftest.py                     196     97    51%   35, 41-43, 48, 54, 60, 66, 72-74, 80-95, 100, 105-107, 113-115, 121-122, 127-128, 133, 139, 148-157, 163-168, 232, 238, 244-250, 258-263, 271-284, 289-295, 302-313, 320-336
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              337    106    69%   39, 50-58, 63-70, 74-75, 79-80, 178, 216-233, 242-250, 254-259, 265, 298-303, 341-348, 391-393, 396, 461-469, 498-499, 505, 508, 544-554, 567-592
	 source_acceptance_test/tests/test_incremental.py       145     20    86%   21-23, 29-31, 36-43, 48-61, 224
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  77     10    87%   15-16, 24-30, 64, 67
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/config_migration.py        23     23     0%   5-37
	 source_acceptance_test/utils/connector_runner.py       112     50    55%   23-26, 32, 36, 39-68, 71-73, 76-78, 81-83, 86-88, 91-93, 96-114, 148-150
	 source_acceptance_test/utils/json_schema_helper.py     105     13    88%   30-31, 38, 41, 65-68, 96, 120, 190-192
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1487    376    75%

Build Passed

Test summary info:

All Passed

erohmensing · 2022-11-09T19:14:18Z

/test connector=connectors/source-chargebee

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

Name                                Stmts   Miss  Cover
-------------------------------------------------------
source_chargebee/source.py             25      0   100%
source_chargebee/__init__.py            2      0   100%
source_chargebee/streams.py           163      9    94%
source_chargebee/utils.py               8      1    88%
source_chargebee/rate_limiting.py      19      7    63%
-------------------------------------------------------
TOTAL                                 217     17    92%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       133      3    98%   87, 93, 230
	 source_acceptance_test/conftest.py                     196     97    51%   35, 41-43, 48, 54, 60, 66, 72-74, 80-95, 100, 105-107, 113-115, 121-122, 127-128, 133, 139, 148-157, 163-168, 232, 238, 244-250, 258-263, 271-284, 289-295, 302-313, 320-336
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              337    106    69%   39, 50-58, 63-70, 74-75, 79-80, 178, 216-233, 242-250, 254-259, 265, 298-303, 341-348, 391-393, 396, 461-469, 498-499, 505, 508, 544-554, 567-592
	 source_acceptance_test/tests/test_incremental.py       145     20    86%   21-23, 29-31, 36-43, 48-61, 224
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  77     10    87%   15-16, 24-30, 64, 67
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/config_migration.py        23     23     0%   5-37
	 source_acceptance_test/utils/connector_runner.py       112     50    55%   23-26, 32, 36, 39-68, 71-73, 76-78, 81-83, 86-88, 91-93, 96-114, 148-150
	 source_acceptance_test/utils/json_schema_helper.py     105     13    88%   30-31, 38, 41, 65-68, 96, 120, 190-192
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1487    376    75%

Build Passed

Test summary info:

All Passed

erohmensing · 2022-11-09T19:25:10Z

docs/connector-development/connector-specification-reference.md


+## Airbyte Modifications to `jsonschema`


Making this more explicit means we can hopefully start enforcing other things if we want to. For example pertaining to enum values, we could also enforce:

if the default keyword is used, there is an item called "default", and the default item is also in the list

that all options are of the type listed

(Ideas from here: https://github.com/airbytehq/airbyte/blob/master/airbyte-integrations/connectors/destination-kafka/src/main/resources/spec.json#L188-L195)

These restrictions AFAIK come from the frontend, so If we wanted to move forward with enforcing more things for better UX up front, I'd say we should be in contact with the frontend team to align on a big list of things to enforce so that it could all be done at once and they know explicitly what they want to enforce, in case that changes.

* Make pytest aware of backward compatibility mark * Unit tests for duplicate values in enum properties * Add docs about airbyte enforcements to enum usage * Add docs info to test

erohmensing added 2 commits November 9, 2022 10:59

Make pytest aware of mark

5a3556c

Unit tests for duplicate values in enum properties

7d84948

erohmensing added 2 commits November 9, 2022 13:10

Add docs about airbyte enforcements to enum usage

3c5aba4

Add docs info to test

643a843

github-actions bot added the area/documentation Improvements or additions to documentation label Nov 9, 2022

erohmensing marked this pull request as ready for review November 9, 2022 18:24

erohmensing requested a review from a team November 9, 2022 19:20

erohmensing commented Nov 9, 2022

View reviewed changes

evantahler approved these changes Nov 9, 2022

View reviewed changes

erohmensing merged commit e73f535 into master Nov 9, 2022

erohmensing deleted the ella/enum-SAT-test branch November 9, 2022 20:47

erohmensing mentioned this pull request Nov 9, 2022

Add acceptance test for correct format on date fields #18741

Closed

DoNotPanicUA mentioned this pull request Nov 11, 2022

Java sources fail after adding a new SAT SPEC test by #19243 #19353

Closed

github-actions bot mentioned this pull request Nov 15, 2022

Bump helm chart version reference to 0.40.43 #19453

Closed

octavia-squidington-iii mentioned this pull request Nov 18, 2022

Bump Airbyte version from 0.40.18 to 0.40.19 #19579

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add SAT test for no duplicates in `enum` properties #19243

Add SAT test for no duplicates in `enum` properties #19243

Uh oh!

erohmensing commented Nov 9, 2022 •

edited

Loading

Uh oh!

github-actions bot commented Nov 9, 2022 •

edited

Loading

❌ Sources (63)

✅ Destinations (0)

Actionable Items

Uh oh!

erohmensing commented Nov 9, 2022 •

edited by github-actions bot

Loading

Uh oh!

erohmensing commented Nov 9, 2022 •

edited by github-actions bot

Loading

Uh oh!

erohmensing commented Nov 9, 2022 •

edited by github-actions bot

Loading

Uh oh!

erohmensing Nov 9, 2022

Uh oh!

Uh oh!

Add SAT test for no duplicates in enum properties #19243

Add SAT test for no duplicates in enum properties #19243

Uh oh!

Conversation

erohmensing commented Nov 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

How

🚨 User Impact 🚨

Pre-merge Checklist

Uh oh!

github-actions bot commented Nov 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Affected Connector Report

❌ Sources (63)

✅ Destinations (0)

Actionable Items

Uh oh!

erohmensing commented Nov 9, 2022 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Passed

Uh oh!

erohmensing commented Nov 9, 2022 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Passed

Uh oh!

erohmensing commented Nov 9, 2022 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Passed

Uh oh!

erohmensing Nov 9, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Add SAT test for no duplicates in `enum` properties #19243

Add SAT test for no duplicates in `enum` properties #19243

erohmensing commented Nov 9, 2022 •

edited

Loading

github-actions bot commented Nov 9, 2022 •

edited

Loading

erohmensing commented Nov 9, 2022 •

edited by github-actions bot

Loading

erohmensing commented Nov 9, 2022 •

edited by github-actions bot

Loading

erohmensing commented Nov 9, 2022 •

edited by github-actions bot

Loading