GA destinations: remove `supportsDbt` and `supportsNormalization` from spec.json #21306

alafanechere · 2023-01-12T11:54:20Z

What

Issue: https://github.com/airbytehq/airbyte-internal-issues/issues/2539
The source of truth for supportsDbt and supportsNormalization is now in the destination definition.
We should remove it from the destination spec.
This PR is doing it for GA destinations.

github-actions · 2023-01-12T11:56:11Z

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 or module version
Add changelog
Publish the new version

✅ Sources (0)

Connector	Version	Changelog	Publish

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

❌ Destinations (6)

Connector	Version	Changelog	Publish
`destination-bigquery`	`2.0.0`	✅	❌ (diff seed version)
`destination-bigquery-denormalized`	`1.2.10`	✅	✅
`destination-databricks`	`0.3.1`	✅	✅
`destination-s3`	`1.0.0`	✅	✅
`destination-s3-glue`	`0.1.1`	✅	✅
`destination-snowflake`	`1.0.0`	✅	❌ (diff seed version)

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

👀 Other Modules (1)

base-normalization

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.

alafanechere · 2023-01-12T14:19:25Z

/test connector=connectors/destination-bigquery

🕑 connectors/destination-bigquery https://github.com/airbytehq/airbyte/actions/runs/3903098782
✅ connectors/destination-bigquery https://github.com/airbytehq/airbyte/actions/runs/3903098782
Python tests coverage:

Name                                                              Stmts   Miss  Cover
-------------------------------------------------------------------------------------
normalization/transform_config/__init__.py                            2      0   100%
normalization/transform_catalog/reserved_keywords.py                 14      0   100%
normalization/transform_catalog/__init__.py                           2      0   100%
normalization/destination_type.py                                    14      0   100%
normalization/__init__.py                                             4      0   100%
normalization/transform_catalog/destination_name_transformer.py     166      8    95%
normalization/transform_catalog/table_name_registry.py              174     34    80%
normalization/transform_config/transform.py                         189     48    75%
normalization/transform_catalog/utils.py                             51     14    73%
normalization/transform_catalog/dbt_macro.py                         22      7    68%
normalization/transform_catalog/catalog_processor.py                147     80    46%
normalization/transform_catalog/transform.py                         61     38    38%
normalization/transform_catalog/stream_processor.py                 595    400    33%
-------------------------------------------------------------------------------------
TOTAL                                                              1441    629    56%

Build Passed

Test summary info:

All Passed

alafanechere · 2023-01-12T14:19:36Z

/test connector=connectors/destination-snowflake

🕑 connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/3903099758
✅ connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/3903099758
Python tests coverage:

Name                                                              Stmts   Miss  Cover
-------------------------------------------------------------------------------------
normalization/transform_config/__init__.py                            2      0   100%
normalization/transform_catalog/reserved_keywords.py                 14      0   100%
normalization/transform_catalog/__init__.py                           2      0   100%
normalization/destination_type.py                                    14      0   100%
normalization/__init__.py                                             4      0   100%
normalization/transform_catalog/destination_name_transformer.py     166      8    95%
normalization/transform_catalog/table_name_registry.py              174     34    80%
normalization/transform_config/transform.py                         189     48    75%
normalization/transform_catalog/utils.py                             51     14    73%
normalization/transform_catalog/dbt_macro.py                         22      7    68%
normalization/transform_catalog/catalog_processor.py                147     80    46%
normalization/transform_catalog/transform.py                         61     38    38%
normalization/transform_catalog/stream_processor.py                 595    400    33%
-------------------------------------------------------------------------------------
TOTAL                                                              1441    629    56%

Build Passed

Test summary info:

All Passed

alafanechere · 2023-01-12T14:19:41Z

/test connector=connectors/destination-s3

🕑 connectors/destination-s3 https://github.com/airbytehq/airbyte/actions/runs/3903100803
✅ connectors/destination-s3 https://github.com/airbytehq/airbyte/actions/runs/3903100803
No Python unittests run

Build Passed

Test summary info:

All Passed

alafanechere · 2023-01-12T14:46:39Z

NB: I don't think it's safe to merge until cloud is deployed with OSS >= 0.43.18

evantahler

Nice!

Major version bump (semver!)
Docs explain why & what version of the platform is needed
connector tests all pass

alafanechere · 2023-01-13T08:12:39Z

Cloud is now on >= 0.43.18.

…c-GA-test

alafanechere · 2023-01-13T08:19:38Z

/publish connector=connectors/destination-bigquery

🕑 Publishing the following connectors:
connectors/destination-bigquery
https://github.com/airbytehq/airbyte/actions/runs/3909580355

Connector	Did it publish?	Were definitions generated?
connectors/destination-bigquery	✅	❌

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

alafanechere · 2023-01-13T08:19:52Z

/publish connector=connectors/destination-s3

🕑 Publishing the following connectors:
connectors/destination-s3
https://github.com/airbytehq/airbyte/actions/runs/3909581722

Connector	Did it publish?	Were definitions generated?
connectors/destination-s3	✅	✅

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

alafanechere · 2023-01-13T08:20:13Z

/publish connector=connectors/destination-snowflake

🕑 Publishing the following connectors:
connectors/destination-snowflake
https://github.com/airbytehq/airbyte/actions/runs/3909584680

Connector	Did it publish?	Were definitions generated?
connectors/destination-snowflake	✅	❌

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

alafanechere · 2023-01-13T08:48:42Z

@airbytehq/use please checkout the changelog.
This will probably make these connectors not work on OSS versions <= 0.43.18.
Feel free to organize a community outreach.

alafanechere · 2023-01-13T10:10:08Z

@evantahler I realize that even if we remove these fields from spec.json they will still be outputted with their protocol default value because Java connectors rely on protocol models to output the spec. It explains why java sources also have these fields set by default.

The BaseConnector.spec() outputs a ConnectorSpecification objects built from the protocol model. So supportsNormalization and supportsDbt default to false and will be present in the spec command output.
Because of Java strict typing and how spec command is implemented I don't think we can easily alter this behavior.
I believe we'll have to make the connector-specific changes in the protocol change PR. #21295

alafanechere · 2023-01-13T10:14:26Z

So the only benefit from the current PR would be confirming the platform 💯 relies on definitions to manage connector normalization. But I'm not sure it's worth the confusion of releasing connectors with supportsNormalization = false in the spec output.

alafanechere · 2023-01-17T09:34:00Z

@davydov-d let me know once you created the OAuth parameters on the Cloud DB and I'll approve the PR so that we can merge it tommorow.

davydov-d · 2023-01-17T15:48:03Z

@davydov-d let me know once you created the OAuth parameters on the Cloud DB and I'll approve the PR so that we can merge it tommorow.

@alafanechere done

alafanechere · 2023-01-25T15:46:33Z

Hey @grishick do you mind if I merge these? It will lead to misleading spec outputs according to what I explained here. I'd understand if you preferred to hold these changes until the protocol update.

grishick · 2023-01-26T00:05:18Z

Hey @grishick do you mind if I merge these? It will lead to misleading spec outputs according to what I explained here. I'd understand if you preferred to hold these changes until the protocol update.

I am confused as to how these changes affect the destinations. From the comment that you linked is sounds like if you merge this PR, normalization will be disabled for BigQuery and Snowflake destinations, which is not good.

alafanechere · 2023-01-26T19:23:48Z

is sounds like if you merge this PR, normalization will be disabled for BigQuery and Snowflake destinations, which is not good.

We now pull the information about the normalization / dbt support from the connector definition, so the connector spec is not used by the platform to determine if normlization is supported. So merging this PR should not break anything. My concern is that the spec output is going to be misleading: as the we remove these field from these connector spec their value will fallback to the default defined in the protocol model. So running spec on BigQuery will output supportsNormalization: false which is not great.
We can hold until the protocol change.

grishick · 2023-01-26T19:44:57Z

So running spec on BigQuery will output supportsNormalization: false which is not great.

This seems dangerous.

Another question. If I am adding a new connector with normalization support - how do I do that without breaking the CI? I cannot add the connector to destination_definitions.yaml or destination_specs.yaml before I publish it, because the CI will complain about a missing Docker image. All integration tests that use normalization will be skipped for this connector, because it has no way of declaring that it supports normalization outside of the seed files.

alafanechere · 2023-02-24T14:23:30Z

Closing to wait for the protocol update tracked here or the decoupling of java connectors from protocol models.

update spec.json

cb05f09

octavia-squidington-iv added area/connectors Connector related issues connectors/destination/snowflake connectors/destination/s3 connectors/source/bigquery labels Jan 12, 2023

bump versions

c994d16

octavia-squidington-iv added area/documentation Improvements or additions to documentation connectors/destination/bigquery labels Jan 12, 2023

update spec.json

aa8d26e

alafanechere marked this pull request as ready for review January 12, 2023 14:19

alafanechere requested review from a team January 12, 2023 14:20

major bump

8e24ac7

evantahler approved these changes Jan 12, 2023

View reviewed changes

alafanechere mentioned this pull request Jan 13, 2023

remove supportsNormalization + supportsDbt from sources spec #21317

Merged

alafanechere added 2 commits January 13, 2023 09:16

Merge branch 'master' into augustin/remove-supports-normalization-spe…

c67a22a

…c-GA-test

update bq changelog

1838b72

auto-bump connector version

0c2144c

octavia-squidington-iii temporarily deployed to more-secrets January 13, 2023 09:07 — with GitHub Actions Inactive

grishick approved these changes Jan 17, 2023

View reviewed changes

alafanechere mentioned this pull request Jan 25, 2023

airbyte-protocol: remove supportsNormalization and supportsDbt from ConnectorSpecification #21295

Closed

alafanechere closed this Feb 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GA destinations: remove `supportsDbt` and `supportsNormalization` from spec.json #21306

GA destinations: remove `supportsDbt` and `supportsNormalization` from spec.json #21306

alafanechere commented Jan 12, 2023 •

edited

Loading

github-actions bot commented Jan 12, 2023 •

edited

Loading

✅ Sources (0)

❌ Destinations (6)

👀 Other Modules (1)

Actionable Items

alafanechere commented Jan 12, 2023 •

edited by github-actions bot

Loading

alafanechere commented Jan 12, 2023 •

edited by github-actions bot

Loading

alafanechere commented Jan 12, 2023 •

edited by github-actions bot

Loading

alafanechere commented Jan 12, 2023 •

edited

Loading

evantahler left a comment

alafanechere commented Jan 13, 2023

alafanechere commented Jan 13, 2023 •

edited by github-actions bot

Loading

alafanechere commented Jan 13, 2023 •

edited by github-actions bot

Loading

alafanechere commented Jan 13, 2023 •

edited by github-actions bot

Loading

alafanechere commented Jan 13, 2023 •

edited

Loading

alafanechere commented Jan 13, 2023

alafanechere commented Jan 13, 2023

alafanechere commented Jan 17, 2023

davydov-d commented Jan 17, 2023

alafanechere commented Jan 25, 2023

grishick commented Jan 26, 2023

alafanechere commented Jan 26, 2023

grishick commented Jan 26, 2023

alafanechere commented Feb 24, 2023

GA destinations: remove supportsDbt and supportsNormalization from spec.json #21306

GA destinations: remove supportsDbt and supportsNormalization from spec.json #21306

Conversation

alafanechere commented Jan 12, 2023 • edited Loading

What

github-actions bot commented Jan 12, 2023 • edited Loading

Affected Connector Report

✅ Sources (0)

❌ Destinations (6)

👀 Other Modules (1)

Actionable Items

alafanechere commented Jan 12, 2023 • edited by github-actions bot Loading

Build Passed

alafanechere commented Jan 12, 2023 • edited by github-actions bot Loading

Build Passed

alafanechere commented Jan 12, 2023 • edited by github-actions bot Loading

Build Passed

alafanechere commented Jan 12, 2023 • edited Loading

evantahler left a comment

Choose a reason for hiding this comment

alafanechere commented Jan 13, 2023

alafanechere commented Jan 13, 2023 • edited by github-actions bot Loading

alafanechere commented Jan 13, 2023 • edited by github-actions bot Loading

alafanechere commented Jan 13, 2023 • edited by github-actions bot Loading

alafanechere commented Jan 13, 2023 • edited Loading

alafanechere commented Jan 13, 2023

alafanechere commented Jan 13, 2023

alafanechere commented Jan 17, 2023

davydov-d commented Jan 17, 2023

alafanechere commented Jan 25, 2023

grishick commented Jan 26, 2023

alafanechere commented Jan 26, 2023

grishick commented Jan 26, 2023

alafanechere commented Feb 24, 2023

GA destinations: remove `supportsDbt` and `supportsNormalization` from spec.json #21306

GA destinations: remove `supportsDbt` and `supportsNormalization` from spec.json #21306

alafanechere commented Jan 12, 2023 •

edited

Loading

github-actions bot commented Jan 12, 2023 •

edited

Loading

alafanechere commented Jan 12, 2023 •

edited by github-actions bot

Loading

alafanechere commented Jan 12, 2023 •

edited by github-actions bot

Loading

alafanechere commented Jan 12, 2023 •

edited by github-actions bot

Loading

alafanechere commented Jan 12, 2023 •

edited

Loading

alafanechere commented Jan 13, 2023 •

edited by github-actions bot

Loading

alafanechere commented Jan 13, 2023 •

edited by github-actions bot

Loading

alafanechere commented Jan 13, 2023 •

edited by github-actions bot

Loading

alafanechere commented Jan 13, 2023 •

edited

Loading