Skip to content

Conversation

@edgao
Copy link
Contributor

@edgao edgao commented Jun 4, 2024

I intend to ship this during assemble (in spite of the large diff, it should actually be a relatively low-risk change; I'll run some tests in the perf test workspace just for extra confidence)

general PR structure:

  • delete RedshiftSqlOperations / RedshiftS3StagingSqlOperations
  • new class: RedshiftStorageOperation. Mostly copied out of RedshiftSqlOperations / RedshiftS3StagingSqlOperations / JdbcSqlOperations
    • plausibly this should be in the cdk?
      • All the final table stuff is completely destination-agnostic (it's identical to snowflake's version, and probably postgres will also be identical)
      • but all the raw table stuff is bespoke per destination
      • IMO we should keep these separate for now - see how postgres goes
    • replaced the jooq sql gen with native kotlin string templating, see e.g. old createTableQueryV2 vs the new RedshiftStorageOperation.Companion.createRawTableQuery
  • new class: RedshiftDV2Migration. Very thin wrapper around the old V1V2Migrator to bring it into the Migrations interface.
  • RedshiftDestination
    • no longer extends AbstractJdbcDestination. We were already overriding all of the superclass methods anyway, and weren't even invoking super.anything()
    • check now runs a mini-sync (similar to what destiantion-snowflake is doing in Destination Snowflake: Adapting to new connector interfaces #38658). Again, we maybe should extract this to CDK.
    • getMigrations now includes RedshiftDV2Migration, so we don't need to handle it separately in getSerializedConsumer
    • further deletions of encrypted staging code
  • Delete the DAT classes, since they rely on a bunch of JDBC base classes. They were already disabled anyway, and we know we want to rewrite these tests to not rely on the container 🤷

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

@vercel
Copy link

vercel bot commented Jun 4, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
airbyte-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 27, 2024 0:02am

Copy link
Contributor Author

edgao commented Jun 4, 2024

@edgao edgao force-pushed the edgao/redshift_test_kotlin_conversion branch from 7c1a4ed to f345c43 Compare June 4, 2024 21:00
@edgao edgao force-pushed the edgao/redshift_new_interfaces branch from 49a3808 to f6685f3 Compare June 4, 2024 21:01
@edgao edgao force-pushed the edgao/redshift_test_kotlin_conversion branch from f345c43 to 2864bef Compare June 5, 2024 15:53
@edgao edgao force-pushed the edgao/redshift_new_interfaces branch from f6685f3 to fd745ce Compare June 5, 2024 15:53
@edgao edgao force-pushed the edgao/redshift_test_kotlin_conversion branch from 2864bef to d0495a9 Compare June 5, 2024 16:03
@edgao edgao force-pushed the edgao/redshift_new_interfaces branch from fd745ce to 5090604 Compare June 5, 2024 16:03
@edgao edgao force-pushed the edgao/redshift_new_interfaces branch from 5090604 to 1b81b3e Compare June 5, 2024 21:07
@edgao edgao changed the base branch from edgao/redshift_test_kotlin_conversion to edgao/redshift_remove_encryption_option June 5, 2024 21:07
@edgao edgao force-pushed the edgao/redshift_new_interfaces branch 2 times, most recently from fb13354 to edf17e1 Compare June 6, 2024 23:50
@edgao edgao changed the base branch from edgao/redshift_remove_encryption_option to edgao/jdbc_gen_id_support June 6, 2024 23:50
@edgao edgao force-pushed the edgao/redshift_new_interfaces branch from edf17e1 to 95fddcf Compare June 6, 2024 23:50
@edgao edgao changed the base branch from edgao/jdbc_gen_id_support to edgao/redshift_remove_encryption_option June 6, 2024 23:50
@edgao edgao force-pushed the edgao/redshift_remove_encryption_option branch from d0273c2 to 3d0e7d3 Compare June 7, 2024 00:01
@edgao edgao force-pushed the edgao/redshift_new_interfaces branch 2 times, most recently from a28e631 to c972810 Compare June 7, 2024 00:06
@edgao edgao force-pushed the edgao/redshift_remove_encryption_option branch from 026cf75 to 2e369fc Compare June 7, 2024 15:15
@edgao edgao force-pushed the edgao/redshift_new_interfaces branch from c972810 to f1c00f7 Compare June 7, 2024 15:15
@edgao edgao force-pushed the edgao/redshift_remove_encryption_option branch from 2e369fc to f7a341e Compare June 7, 2024 16:33
@edgao edgao force-pushed the edgao/redshift_new_interfaces branch 2 times, most recently from 9511622 to c8dfb6c Compare June 7, 2024 21:35
@edgao edgao marked this pull request as ready for review June 21, 2024 21:10
@edgao edgao requested a review from a team as a code owner June 21, 2024 21:10
@octavia-squidington-iv octavia-squidington-iv requested a review from a team June 21, 2024 21:11
@edgao edgao force-pushed the edgao/redshift_new_interfaces branch from 1c20def to 1750b55 Compare June 25, 2024 17:56
@edgao edgao force-pushed the edgao/redshift_remove_encryption_option branch from 4cb52bc to b7434d0 Compare June 25, 2024 22:58
@edgao edgao force-pushed the edgao/redshift_new_interfaces branch from 1750b55 to 2f51fd9 Compare June 25, 2024 22:58
@edgao edgao force-pushed the edgao/redshift_remove_encryption_option branch from b7434d0 to cd415ea Compare June 26, 2024 16:33
Base automatically changed from edgao/redshift_remove_encryption_option to master June 26, 2024 16:54
@edgao edgao force-pushed the edgao/redshift_new_interfaces branch 3 times, most recently from ea7ac8e to b22f4d9 Compare June 26, 2024 20:56
@octavia-squidington-iii octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Jun 26, 2024
@edgao edgao force-pushed the edgao/redshift_new_interfaces branch from b22f4d9 to 4d34b0a Compare June 26, 2024 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/destination/redshift

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants