Destination Redshift: deprecate old migration Java code #25698

cynthiaxyin · 2023-04-29T00:18:49Z

What

Functionality was added over a year ago (#12064) to migrate Redshift tables from VARCHAR type to SUPER type. Tables have now been SUPER type by default for awhile, so we are removing this old migration code that is no longer needed.

How

Delete migration-related Java code. Normalization code will be deleted in #25771.

🚨 User Impact 🚨

The set of impacted users is expected to be very small: anyone who has been running old versions of Redshift and Airbyte for over a year and is still using VARCHAR tables with likely stale data. Anyone who is impacted can manually migrate their Redshift tables if needed. The team voted to remove this functionality as it was implemented over a year ago.

Pre-merge Actions

Expand the relevant checklist and delete the others.

Updating a connector

Community member or Airbyter

Grant edit access to maintainers (instructions)
Unit & integration tests added

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
If new credentials are required for use in CI, add them to GSM. Instructions.

github-actions · 2023-04-29T00:19:10Z

github-actions · 2023-04-29T00:22:36Z

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 (23)

Connector	Version	Changelog	Publish
`destination-azure-blob-storage`	`0.2.0`	✅	✅
`destination-clickhouse`	`0.2.3`	✅	✅
`destination-clickhouse-strict-encrypt`	`0.2.3`	🔵 (ignored)	🔵 (ignored)
`destination-databricks`	`1.0.2`	✅	✅
`destination-dynamodb`	`0.1.7`	✅	✅
`destination-exasol`	`0.1.1`	✅	✅
`destination-gcs`	`0.2.17`	❌ (changelog missing)	✅
`destination-mariadb-columnstore`	`0.1.7`	✅	✅
`destination-mssql`	`0.1.23`	✅	✅
`destination-mssql-strict-encrypt`	`0.1.23`	🔵 (ignored)	🔵 (ignored)
`destination-mysql`	`0.1.20`	✅	✅
`destination-mysql-strict-encrypt`	❌ `0.1.21` (mismatch: `0.1.20`)	🔵 (ignored)	🔵 (ignored)
`destination-oracle`	`0.1.19`	✅	✅
`destination-oracle-strict-encrypt`	`0.1.19`	🔵 (ignored)	🔵 (ignored)
`destination-postgres`	`0.3.27`	✅	✅
`destination-postgres-strict-encrypt`	`0.3.27`	🔵 (ignored)	🔵 (ignored)
`destination-redshift`	`0.4.7`	✅	✅
`destination-rockset`	`0.1.4`	✅	✅
`destination-snowflake`	`0.4.63`	✅	✅
`destination-starburst-galaxy`	`0.0.1`	✅	✅
`destination-teradata`	`0.1.1`	✅	✅
`destination-tidb`	`0.1.1`	✅	✅
`destination-yugabytedb`	`0.1.1`	✅	✅

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.

cynthiaxyin · 2023-05-01T21:39:37Z

/test connector=connectors/destination-redshift

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

Name                                                              Stmts   Miss  Cover
-------------------------------------------------------------------------------------
normalization/transform_config/__init__.py                            2      0   100%
normalization/transform_catalog/reserved_keywords.py                 15      0   100%
normalization/transform_catalog/__init__.py                           2      0   100%
normalization/destination_type.py                                    18      0   100%
normalization/__init__.py                                             4      0   100%
normalization/transform_catalog/destination_name_transformer.py     171     10    94%
normalization/transform_catalog/table_name_registry.py              174     34    80%
normalization/transform_config/transform.py                         195     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                         65     39    40%
normalization/transform_catalog/stream_processor.py                 595    400    33%
-------------------------------------------------------------------------------------
TOTAL                                                              1461    632    57%

Build Passed

Test summary info:

All Passed

cynthiaxyin · 2023-05-01T21:46:13Z

...jdbc/src/main/java/io/airbyte/integrations/destination/jdbc/JdbcBufferedConsumerFactory.java

-        sqlOperations.onDestinationCloseOperations(database, writeConfigs);
-      }
-    };
+  private static OnCloseFunction onCloseFunction() {


This is not actually doing anything, but leaving it to not break any functionality that expects a non-null OnCloseFunction.

cynthiaxyin · 2023-05-01T22:08:03Z

airbyte-integrations/connectors/destination-redshift/Dockerfile

@@ -16,5  16,5 @@ ENV APPLICATION destination-redshift

 COPY --from=build /airbyte /airbyte

-LABEL io.airbyte.version=0.4.6
+LABEL io.airbyte.version=0.4.7


Not sure if this is right.

This seems right since this is a bug fix but it's not a user-facing functionality in a backwards compatible manner

Ah ok I wasn't sure if this actually fell under a major version change

data that was previously being synced will no longer be synced

If anything the checkpointing changes for JDBC connectors should have been a major version bump 😰

cynthiaxyin · 2023-05-01T22:08:31Z

...va/io/airbyte/integrations/destination/redshift/RedshiftInsertDestinationAcceptanceTest.java

-import org.jooq.Record;
-import org.jooq.Result;
-import org.junit.jupiter.api.Disabled;
-import org.junit.jupiter.api.Test;

 /**
 * Integration test testing the {@link RedshiftInsertDestination}. As the Redshift test credentials
 * contain S3 credentials by default, we remove these credentials.
 */
 public class RedshiftInsertDestinationAcceptanceTest extends RedshiftStagingS3DestinationAcceptanceTest {


Do we still need this file and RedshiftS3StagingInsertDestinationAcceptanceTest.java below? There are no tests inside anymore.

Yes, because they will execute the tests using the config.json tests for RedshiftInsertDestinationAcceptanceTest, as for RedshiftS3StagingInsertDestinationAcceptanceTest that doesn't seem necessary since the same config of config_staging.json will be used within RedshiftStagingS3Destination

/rant RedshiftInsert should not be extending from RedshiftStagingS3

🤷‍♀️ ok I'll leave RedshiftS3StagingInsertDestinationAcceptanceTest to mirror RedshiftInsertDestinationAcceptanceTest to avoid confusion why there's one and not the other.

ryankfu

LGTM

cynthiaxyin · 2023-05-02T17:02:44Z

/publish connector=connectors/destination-redshift

cynthiaxyin · 2023-05-02T18:28:12Z

/publish connector=connectors/destination-redshift

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

Connector	Version	Did it publish?	Were definitions generated?
connectors/destination-redshift	0.4.7	✅	✅

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

) * first pass * update changelog * auto-bump connector version * bump metadata.yaml --------- Co-authored-by: Octavia Squidington III <[email protected]>

octavia-squidington-iii added area/connectors Connector related issues connectors/destination/redshift labels Apr 29, 2023

This comment was marked as outdated.

Sign in to view

This comment was marked as off-topic.

Sign in to view

cynthiaxyin changed the title ~~Deprecate old Redshift migration code~~ Deprecate old Redshift migration Java code May 1, 2023

cynthiaxyin changed the title ~~Deprecate old Redshift migration Java code~~ Destination Redshift: deprecate old migration Java code May 1, 2023

first pass

8d79525

cynthiaxyin force-pushed the cynthia/destination_redshift_migration_deprecate branch from 072b38c to 0547322 Compare May 1, 2023 21:39

octavia-squidington-iii added the area/documentation Improvements or additions to documentation label May 1, 2023

cynthiaxyin marked this pull request as ready for review May 1, 2023 22:11

cynthiaxyin requested a review from a team as a code owner May 1, 2023 22:11

cynthiaxyin commented May 1, 2023

View reviewed changes

update changelog

7929287

cynthiaxyin force-pushed the cynthia/destination_redshift_migration_deprecate branch from 0547322 to 7929287 Compare May 1, 2023 22:31

ryankfu approved these changes May 2, 2023

View reviewed changes

octavia-squidington-iii and others added 2 commits May 2, 2023 18:57

auto-bump connector version

1aa485d

bump metadata.yaml

8b87013

cynthiaxyin enabled auto-merge (squash) May 2, 2023 19:29

cynthiaxyin merged commit 74a3e2a into master May 2, 2023

cynthiaxyin deleted the cynthia/destination_redshift_migration_deprecate branch May 2, 2023 19:41

cynthiaxyin mentioned this pull request May 2, 2023

Destination Redshift: deprecate old migration normalization code #25771

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Destination Redshift: deprecate old migration Java code #25698

Destination Redshift: deprecate old migration Java code #25698

cynthiaxyin commented Apr 29, 2023 •

edited

Loading

github-actions bot commented Apr 29, 2023 •

edited by cynthiaxyin

Loading

This comment was marked as outdated.

github-actions bot commented Apr 29, 2023 •

edited

Loading

✅ Sources (0)

❌ Destinations (23)

👀 Other Modules (1)

Actionable Items

This comment was marked as off-topic.

This comment was marked as off-topic.

cynthiaxyin commented May 1, 2023 •

edited by github-actions bot

Loading

cynthiaxyin May 1, 2023

cynthiaxyin May 1, 2023

ryankfu May 2, 2023

cynthiaxyin May 2, 2023

ryankfu May 2, 2023

cynthiaxyin May 1, 2023

ryankfu May 2, 2023

cynthiaxyin May 2, 2023

ryankfu left a comment

cynthiaxyin commented May 2, 2023

cynthiaxyin commented May 2, 2023 •

edited by github-actions bot

Loading

Destination Redshift: deprecate old migration Java code #25698

Destination Redshift: deprecate old migration Java code #25698

Conversation

cynthiaxyin commented Apr 29, 2023 • edited Loading

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Actions

Community member or Airbyter

Airbyter

github-actions bot commented Apr 29, 2023 • edited by cynthiaxyin Loading

Before Merging a Connector Pull Request

This comment was marked as outdated.

github-actions bot commented Apr 29, 2023 • edited Loading

Affected Connector Report

✅ Sources (0)

❌ Destinations (23)

👀 Other Modules (1)

Actionable Items

This comment was marked as off-topic.

This comment was marked as off-topic.

cynthiaxyin commented May 1, 2023 • edited by github-actions bot Loading

Build Passed

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

ryankfu left a comment

Choose a reason for hiding this comment

cynthiaxyin commented May 2, 2023

cynthiaxyin commented May 2, 2023 • edited by github-actions bot Loading

cynthiaxyin commented Apr 29, 2023 •

edited

Loading

github-actions bot commented Apr 29, 2023 •

edited by cynthiaxyin

Loading

github-actions bot commented Apr 29, 2023 •

edited

Loading

cynthiaxyin commented May 1, 2023 •

edited by github-actions bot

Loading

cynthiaxyin commented May 2, 2023 •

edited by github-actions bot

Loading