Destinations: Refreshes: CDK updates #38067

edgao · 2024-05-08T18:24:10Z

closes https://github.com/airbytehq/airbyte-internal-issues/issues/7606

These are the changes that break platform compatibility. We shouldn't release this until platform is ready for it, and we can turn on the supportsRefreshes metadata in destinations.

vercel · 2024-05-08T18:24:14Z

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		Jun 12, 2024 9:45pm

edgao · 2024-05-08T21:13:32Z

Destinations: Refreshes: CDK updates #38067 : 2 dependent PRs (#38086 , #38622 ) 👈
Destination bigquery: Bump cdk again #38331
Destinations: Refreshes: Track stream statuses in async framework #38075
Destinations CDK: CatalogParser sets default namespace #38121 : 1 other dependent PR (#39107 )
Destination bigquery: switch cdk back to artifact #38770
master

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @edgao and the rest of your teammates on Graphite

edgao · 2024-05-08T21:15:21Z

.../io/airbyte/integrations/base/destination/typing_deduping/BaseSqlGeneratorIntegrationTest.kt

                DestinationSyncMode.APPEND_DEDUP,
                primaryKey,
                Optional.of(cursor),
-                COLUMNS
+                COLUMNS,
+                0,


maybe we should use nonzero generation id, so that tests are slightly more stringent?

edgao · 2024-05-08T21:16:31Z

...t/kotlin/io/airbyte/integrations/base/destination/typing_deduping/DefaultTyperDeduperTest.kt

                DestinationSyncMode.OVERWRITE,
                mock(),
                mock(),
-                mock()
+                mock(),
+                0,


for a later stacked PR: I think we need to add generation-related logic into DefaultTyperDeduper? so we'll want more test cases in this class...

edgao · 2024-05-08T21:34:36Z

...ing/src/main/kotlin/io/airbyte/integrations/base/destination/typing_deduping/StreamConfig.kt

 import java.util.*
 import kotlin.collections.LinkedHashMap

 data class StreamConfig(
    val id: StreamId,
-    val syncMode: SyncMode,


this is the source sync mode, which we should never look at. Delete it.

edgao · 2024-06-03T16:53:41Z

...ng/src/main/kotlin/io/airbyte/integrations/base/destination/typing_deduping/CatalogParser.kt

@airbytehq/destinations wdyt about:

add a constructor param supportsRefreshes: Boolean = false

add if (!supportsRefreshes) { throw IllegalArgumentException() } to the constructor

this would mean:

connectors have to explicitly opt into the new behavior

there's no way to accidentally release a connector without refreshes support, but using a new CDK version

I don't think our tests would catch this currently, since we're using expectedrecords.jsonl files defined in each connector?

What happens to the existing connectors which still want to move to new CDK but refreshes not yet implemented ?

they don't :P

do we want to build full backwards-compat logic? I think that's doable, just kind of annoying. And we already want to build refresh support in all our certified connectors, so... do we care strongly about the rest?

backwards compat would roughly require:

AsyncStreamConsumer needs to toggle default stream status

CatalogParser toggles the please upgrade platform error and sync mode override

AbstractStreamOp toggles between new/old behavior (or we just create a separate AbstractNonRefreshStreamOp, idk - it's sufficiently different that I don't know how much makes sense to share)

... tests are probably really annoying

octavia-squidington-iii added the CDK Connector Development Kit label May 8, 2024

edgao force-pushed the destinations_refreshes_cdk_changes branch 3 times, most recently from 9f8ae69 to 50c1e4b Compare May 8, 2024 18:40

edgao mentioned this pull request May 8, 2024

Destinations: Refreshes: Track stream statuses in async framework #38075

Merged

edgao force-pushed the destinations_refreshes_cdk_changes branch from 50c1e4b to e857ce6 Compare May 8, 2024 21:13

edgao commented May 8, 2024

View reviewed changes

edgao force-pushed the destinations_refreshes_cdk_changes branch from e857ce6 to 982948e Compare May 8, 2024 21:21

edgao commented May 8, 2024

View reviewed changes

edgao force-pushed the destinations_refreshes_cdk_changes branch 6 times, most recently from 6839b3f to d361d45 Compare May 8, 2024 22:00

edgao requested a review from gisripa May 8, 2024 23:00

edgao mentioned this pull request May 9, 2024

Destinations CDK: Set default namespace lower in the async framework call stack #38086

Closed

edgao force-pushed the destinations_refreshes_cdk_changes branch 3 times, most recently from a3bb96c to eca8010 Compare May 10, 2024 17:10

edgao changed the base branch from master to edgao/catalog_parser_improvements May 10, 2024 17:10

edgao mentioned this pull request May 10, 2024

Destinations CDK: CatalogParser sets default namespace #38121

Merged

edgao force-pushed the edgao/catalog_parser_improvements branch from 7aff2a5 to 2047aec Compare May 10, 2024 17:18

edgao force-pushed the destinations_refreshes_cdk_changes branch from eca8010 to 7058132 Compare May 10, 2024 17:18

edgao force-pushed the edgao/catalog_parser_improvements branch from 2047aec to 2f4b6ac Compare May 10, 2024 17:33

edgao force-pushed the destinations_refreshes_cdk_changes branch from 7058132 to 1306644 Compare May 10, 2024 17:33

gisripa mentioned this pull request May 10, 2024

redshift_catalog_parser_update #38126

Merged

2 tasks

edgao force-pushed the destinations_refreshes_cdk_changes branch from 05d9bec to dd63e36 Compare May 30, 2024 21:21

edgao force-pushed the edgao/bigquery_more_cdk_bump branch from 0dec7ae to 52d7ac0 Compare May 31, 2024 15:20

edgao force-pushed the destinations_refreshes_cdk_changes branch 2 times, most recently from 2c017cc to 5c99a98 Compare June 3, 2024 16:46

octavia-squidington-iv requested a review from a team June 3, 2024 16:48

edgao commented Jun 3, 2024

View reviewed changes

edgao force-pushed the edgao/bigquery_more_cdk_bump branch from 52d7ac0 to 5304f0e Compare June 6, 2024 16:16

edgao force-pushed the destinations_refreshes_cdk_changes branch from 5c99a98 to cd81f59 Compare June 6, 2024 16:16

edgao force-pushed the edgao/bigquery_more_cdk_bump branch from 5304f0e to 2b5001c Compare June 6, 2024 16:23

edgao force-pushed the destinations_refreshes_cdk_changes branch from cd81f59 to cf625ed Compare June 6, 2024 16:23

edgao force-pushed the edgao/bigquery_more_cdk_bump branch from 2b5001c to 92b3ce9 Compare June 6, 2024 22:00

edgao force-pushed the destinations_refreshes_cdk_changes branch from cf625ed to b22e6e0 Compare June 6, 2024 22:00

edgao force-pushed the edgao/bigquery_more_cdk_bump branch from 92b3ce9 to eef7413 Compare June 6, 2024 22:24

edgao force-pushed the destinations_refreshes_cdk_changes branch from b22e6e0 to 9dd4a9d Compare June 6, 2024 22:24

edgao force-pushed the edgao/bigquery_more_cdk_bump branch from eef7413 to eed66f8 Compare June 7, 2024 23:12

edgao force-pushed the destinations_refreshes_cdk_changes branch from 9dd4a9d to ea03c9b Compare June 7, 2024 23:13

edgao force-pushed the edgao/bigquery_more_cdk_bump branch from eed66f8 to da3e829 Compare June 10, 2024 16:13

edgao force-pushed the destinations_refreshes_cdk_changes branch from ea03c9b to f00bab9 Compare June 10, 2024 16:13

edgao force-pushed the edgao/bigquery_more_cdk_bump branch from da3e829 to 39971f7 Compare June 10, 2024 16:41

edgao force-pushed the destinations_refreshes_cdk_changes branch from f00bab9 to 0850f44 Compare June 10, 2024 16:41

edgao force-pushed the edgao/bigquery_more_cdk_bump branch from 39971f7 to 292aa89 Compare June 10, 2024 17:02

edgao force-pushed the destinations_refreshes_cdk_changes branch from 0850f44 to e7f351c Compare June 10, 2024 17:02

edgao force-pushed the edgao/bigquery_more_cdk_bump branch from 292aa89 to bc71af7 Compare June 10, 2024 17:03

edgao force-pushed the destinations_refreshes_cdk_changes branch from e7f351c to 5f84378 Compare June 10, 2024 17:04

octavia-squidington-iii removed the area/connectors Connector related issues label Jun 10, 2024

Base automatically changed from edgao/bigquery_more_cdk_bump to master June 10, 2024 18:35

gisripa force-pushed the destinations_refreshes_cdk_changes branch from 5f84378 to 8234563 Compare June 11, 2024 00:03

edgao force-pushed the destinations_refreshes_cdk_changes branch from 8234563 to 21ed84f Compare June 12, 2024 17:40

throw exception on bad generation id

12be697

gisripa force-pushed the destinations_refreshes_cdk_changes branch from 21ed84f to 12be697 Compare June 12, 2024 21:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Destinations: Refreshes: CDK updates #38067

Destinations: Refreshes: CDK updates #38067

edgao commented May 8, 2024 •

edited

vercel bot commented May 8, 2024 •

edited

edgao commented May 8, 2024 •

edited

edgao May 8, 2024

edgao May 8, 2024

edgao May 8, 2024

edgao Jun 3, 2024

gisripa Jun 3, 2024

edgao Jun 3, 2024

Destinations: Refreshes: CDK updates #38067

Are you sure you want to change the base?

Destinations: Refreshes: CDK updates #38067

Conversation

edgao commented May 8, 2024 • edited

vercel bot commented May 8, 2024 • edited

edgao commented May 8, 2024 • edited

edgao May 8, 2024

Choose a reason for hiding this comment

edgao May 8, 2024

Choose a reason for hiding this comment

edgao May 8, 2024

Choose a reason for hiding this comment

edgao Jun 3, 2024

Choose a reason for hiding this comment

gisripa Jun 3, 2024

Choose a reason for hiding this comment

edgao Jun 3, 2024

Choose a reason for hiding this comment

edgao commented May 8, 2024 •

edited

vercel bot commented May 8, 2024 •

edited

edgao commented May 8, 2024 •

edited