DV2 TyperDeduper: Extract migrations to separate method #35376

edgao · 2024-02-16T21:55:30Z

Based on top of #35342

Coincidentally, closes https://github.com/airbytehq/airbyte-internal-issues/issues/2469. After this PR, we'll trigger soft resets after raw table creation.

Extract some common code from the DefaultTyperDeduper and NoopTyperDeduperWithMigrations classes. This is just a refactor to simplify some future work.

It also fixes some tests in DefaultTyperDeduperTest. Previously some tests weren't actually doing anything, because they called clearInvocations() followed immediately by verifyNoMoreInteractions(), which is effectively just assertTrue(true).

The CDK build.gradle changes are just copied out of #34637. Once that's merged, there should be no diff with master.

vercel · 2024-02-16T21:55:38Z

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		Feb 28, 2024 7:13pm

edgao · 2024-02-16T23:44:56Z

Current dependencies on/for this PR:

Java CDK: Staging destinations include timezone in extracted_at in CSV file #35313 : 4 dependent PRs (#35308 , #35315 , #35316 and 1 other)
DV2 destinations: Build DestinationState / Migration framework #35303
DV2 TyperDeduper: Extract migrations to separate method #35376 👈
Destinations CDK: Refactor T+D to gather required world state upfront #35342 : 3 other dependent PRs (#35354 , #35385 , #35456 )
master

This stack of pull requests is managed by Graphite.

edgao · 2024-02-21T21:54:25Z

.../main/java/io/airbyte/integrations/base/destination/typing_deduping/DefaultTyperDeduper.java

    if (overwriteStreamsWithTmpTable != null) {
      throw new IllegalStateException("Tables were already prepared.");
    }
    overwriteStreamsWithTmpTable = ConcurrentHashMap.newKeySet();
    LOGGER.info("Preparing tables");

-    // This is intentionally not done in parallel to avoid rate limits in some destinations.
-    prepareSchemas(parsedCatalog);


this call now happens in prepareSchemasAndRawTables

edgao · 2024-02-21T21:54:45Z

.../main/java/io/airbyte/integrations/base/destination/typing_deduping/DefaultTyperDeduper.java

-    // This is intentionally not done in parallel to avoid rate limits in some destinations.
-    prepareSchemas(parsedCatalog);
-
-    // TODO: Either the migrations run the soft reset and create v2 tables or the actual prepare tables.


this logic now happens in TyperDeduperUtil.executeRawTableMigrations

edgao · 2024-02-21T21:57:02Z

...g/src/main/java/io/airbyte/integrations/base/destination/typing_deduping/TyperDeduperUtil.kt

+import java.util.concurrent.ExecutorService
+
+
+class TyperDeduperUtil {


this class is partly taken from @jbfbell 's https://github.com/airbytehq/airbyte/pull/34637/files#diff-c9e7bf1074e965f63b135422dd2a7ac6ba196046b226a0ffe3a855a4eb82ed3b

it'll have some merge conflicts b/c I reordered the arguments in prepareAllSchemas but 🤷 just wanted to be consistent across the methods, and I generally prefer infra-ish params (executorservice, destinationhandler, etc.) to come before functional-ish params (parsedcatalog, streamconfig, etc.)

edgao · 2024-02-21T22:05:54Z

...g/src/main/java/io/airbyte/integrations/base/destination/typing_deduping/TyperDeduperUtil.kt

+      // with current state of raw tables & final tables. This is done first before gather initial state
+      // to avoid recreating
+      // final tables later again.
+      val runMigrationsResult =


as noted above - this code originally lived in DefaultTyperDeduper, I just extracted it to a separate class.

edgao · 2024-02-21T22:07:26Z

...t/java/io/airbyte/integrations/base/destination/typing_deduping/DefaultTyperDeduperTest.java

@@ -161,12 +165,14 @@ void existingEmptyTableMatchingSchema() throws Exception {
    initialStates.forEach(initialState -> {
      when(initialState.isFinalTablePresent()).thenReturn(true);
      when(initialState.isFinalTableEmpty()).thenReturn(true);
-      when(initialState.isSchemaMismatch()).thenReturn(true);
+      when(initialState.isSchemaMismatch()).thenReturn(false);


this test previously didn't actually assert anything, and was secretly broken. This is the correct version.

(specifically: if there are no final tables, isSchemaMismatch is supposed to return false)

Signed-off-by: Gireesh Sreepathi <gisripa@gmail.com>

gisripa · 2024-02-28T19:45:41Z

/publish-java-cdk

🕑 https://github.com/airbytehq/airbyte/actions/runs/8086198897
✅ Successfully published Java CDK version=0.23.7!

octavia-squidington-iii added the CDK Connector Development Kit label Feb 16, 2024

edgao force-pushed the edgao/typer_deduper_interface_cleanup branch 2 times, most recently from 796b41f to 98802c6 Compare February 16, 2024 23:44

edgao changed the base branch from master to gireesh/02-15-cdk/td-init-state February 16, 2024 23:44

This was referenced Feb 16, 2024

Destinations CDK: Refactor T+D to gather required world state upfront #35342

Merged

Destination Redshift: CDK T+D initial state refactor #35354

Merged

gisripa mentioned this pull request Feb 17, 2024

Destination Postgres: CDK T+D initial state gathering #35385

Merged

edgao force-pushed the edgao/typer_deduper_interface_cleanup branch from e596cd7 to d0fbcd8 Compare February 17, 2024 01:21

gisripa mentioned this pull request Feb 20, 2024

Destination Snowflake: CDK T+D initial state refactor #35456

Merged

gisripa force-pushed the gireesh/02-15-cdk/td-init-state branch from fb3ce67 to 52ef29f Compare February 21, 2024 02:59

edgao force-pushed the edgao/typer_deduper_interface_cleanup branch from e282bf3 to 71904f4 Compare February 21, 2024 20:35

edgao changed the title ~~Extract migrations to separate method~~ DV2 TyperDeduper: Extract migrations to separate method Feb 21, 2024

edgao force-pushed the edgao/typer_deduper_interface_cleanup branch 2 times, most recently from 829841d to dcb79ff Compare February 21, 2024 21:52

edgao commented Feb 21, 2024

View reviewed changes

edgao force-pushed the edgao/typer_deduper_interface_cleanup branch from dcb79ff to ff34c69 Compare February 21, 2024 21:58

edgao commented Feb 21, 2024

View reviewed changes

edgao force-pushed the edgao/typer_deduper_interface_cleanup branch from ff34c69 to d2c07e5 Compare February 21, 2024 22:13

edgao marked this pull request as ready for review February 21, 2024 22:38

edgao requested a review from a team as a February 21, 2024 22:38

gisripa force-pushed the gireesh/02-15-cdk/td-init-state branch from d84938e to 71d4578 Compare February 21, 2024 23:45

edgao force-pushed the edgao/typer_deduper_interface_cleanup branch from d2c07e5 to d307cb7 Compare February 22, 2024 00:36

gisripa force-pushed the gireesh/02-15-cdk/td-init-state branch 2 times, most recently from 2aab345 to 16843df Compare February 22, 2024 22:07

Base automatically changed from gireesh/02-15-cdk/td-init-state to master February 22, 2024 23:00

edgao force-pushed the edgao/typer_deduper_interface_cleanup branch 2 times, most recently from 06ac33d to 90b10f3 Compare February 22, 2024 23:45

wild idea

d6199c6

edgao force-pushed the edgao/typer_deduper_interface_cleanup branch from 90b10f3 to d6199c6 Compare February 23, 2024 21:12

jbfbell approved these changes Feb 27, 2024

View reviewed changes

gisripa force-pushed the edgao/typer_deduper_interface_cleanup branch from d6199c6 to f0a9727 Compare February 27, 2024 22:53

gisripa changed the title ~~DV2 TyperDeduper: Extract migrations to separate method~~ wild idea Feb 27, 2024

gisripa force-pushed the edgao/typer_deduper_interface_cleanup branch from f0a9727 to c7e2466 Compare February 27, 2024 22:54

edgao force-pushed the edgao/typer_deduper_interface_cleanup branch from c7e2466 to d6199c6 Compare February 27, 2024 22:57

edgao changed the title ~~wild idea~~ DV2 TyperDeduper: Extract migrations to separate method wild idea Feb 27, 2024

gisripa changed the title ~~DV2 TyperDeduper: Extract migrations to separate method wild idea~~ DV2 TyperDeduper: Extract migrations to separate method Feb 28, 2024

gisripa added 4 commits February 28, 2024 10:52

nit: rename method

f946fb9

Signed-off-by: Gireesh Sreepathi <gisripa@gmail.com>

Merge branch 'master' into edgao/typer_deduper_interface_cleanup

f5e878e

cdk version bump

7a35038

Signed-off-by: Gireesh Sreepathi <gisripa@gmail.com>

Merge branch 'master' into edgao/typer_deduper_interface_cleanup

189e32e

gisripa merged commit f0f7a98 into master Feb 28, 2024
26 checks passed

gisripa deleted the edgao/typer_deduper_interface_cleanup branch February 28, 2024 22:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DV2 TyperDeduper: Extract migrations to separate method #35376

DV2 TyperDeduper: Extract migrations to separate method #35376

edgao commented Feb 16, 2024 •

edited

Loading

vercel bot commented Feb 16, 2024 •

edited

Loading

edgao commented Feb 16, 2024 •

edited

Loading

edgao Feb 21, 2024

edgao Feb 21, 2024

edgao Feb 21, 2024

edgao Feb 21, 2024

edgao Feb 21, 2024

gisripa commented Feb 28, 2024 •

edited by github-actions bot

Loading

		import java.util.concurrent.ExecutorService


		class TyperDeduperUtil {

DV2 TyperDeduper: Extract migrations to separate method #35376

DV2 TyperDeduper: Extract migrations to separate method #35376

Conversation

edgao commented Feb 16, 2024 • edited Loading

vercel bot commented Feb 16, 2024 • edited Loading

edgao commented Feb 16, 2024 • edited Loading

edgao Feb 21, 2024

Choose a reason for hiding this comment

edgao Feb 21, 2024

Choose a reason for hiding this comment

edgao Feb 21, 2024

Choose a reason for hiding this comment

edgao Feb 21, 2024

Choose a reason for hiding this comment

edgao Feb 21, 2024

Choose a reason for hiding this comment

gisripa commented Feb 28, 2024 • edited by github-actions bot Loading

edgao commented Feb 16, 2024 •

edited

Loading

vercel bot commented Feb 16, 2024 •

edited

Loading

edgao commented Feb 16, 2024 •

edited

Loading

gisripa commented Feb 28, 2024 •

edited by github-actions bot

Loading