Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Destination mysql: Switch to dv2 raw tables #37322

Closed
wants to merge 1 commit into from

Conversation

edgao
Copy link
Contributor

@edgao edgao commented Apr 15, 2024

Switch the raw tables to DV2 format. See #36936 for implementing T+D. This PR sets a few "disable T+D" booleans for the sake of being able to run tests; that PR unsets them.

I'll merge #36936 into this branch and release them as a single major version bump - not planning to publish a version of destination-mysql with just dv2 raw tables.

There's some weird stuff going on in MysqlNameTransformer, which I didn't try to solve here. I mostly just don't understand what all the methods are supposed to do (e.g. how does getIdentifier relate to convertStreamName?). MysqlNameTransformer behaves differently from PostgresNameTransformer, and I elected to work around that inconsistency. IMO we should probably revamp that class to be less opinionated about what tables we create.

Copy link

vercel bot commented Apr 15, 2024

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 Apr 29, 2024 3:11pm

Copy link
Contributor Author

edgao commented Apr 15, 2024

@edgao edgao mentioned this pull request Apr 15, 2024
@@ -132,23 +136,22 @@ protected List<JsonNode> retrieveRecords(final TestDestinationEnv testEnv,
}

private List<JsonNode> retrieveRecordsFromTable(final String tableName, final String schemaName) throws SQLException {
try (final DSLContext dslContext = DSLContextFactory.create(
final DSLContext dslContext = DSLContextFactory.create(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think jooq removed the autocloseable interface from DslContext?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah had the same experience in Oracle

@edgao edgao force-pushed the dv2/td_tests_allow_unsafe_cast_2 branch from 94a3dc0 to dd57516 Compare April 15, 2024 17:33
@edgao edgao force-pushed the dv2/td_tests_allow_unsafe_cast_2 branch from dd57516 to 97d1fa3 Compare April 15, 2024 19:48
@edgao edgao force-pushed the dv2/mysql_raw_tables branch 2 times, most recently from a8dc68e to 0a27153 Compare April 15, 2024 22:10
@edgao edgao marked this pull request as ready for review April 15, 2024 22:11
@edgao edgao requested a review from a team as a code owner April 15, 2024 22:11
@octavia-squidington-iv octavia-squidington-iv requested review from a team April 15, 2024 22:12
@edgao edgao force-pushed the dv2/td_tests_allow_unsafe_cast_2 branch from 97d1fa3 to 4e09b56 Compare April 15, 2024 22:57
@edgao edgao force-pushed the dv2/mysql_raw_tables branch 2 times, most recently from bd83d63 to f6e353f Compare April 16, 2024 16:25
@@ -43,6 +43,7 @@
import org.junit.jupiter.api.Timeout;
import org.testcontainers.containers.MySQLContainer;

@Disabled
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

everything starts failing because we're writing to a different raw table. Disable the legacy DAT suite.

... https://github.com/airbytehq/airbyte/issues/29818 continues to exist

@edgao edgao force-pushed the dv2/mysql_raw_tables branch 3 times, most recently from 4255c6e to f3ee677 Compare April 16, 2024 21:07
@edgao edgao marked this pull request as draft April 17, 2024 15:35
@edgao edgao force-pushed the dv2/td_tests_allow_unsafe_cast_2 branch from 41a159a to ecffdd8 Compare April 22, 2024 23:09
@edgao edgao force-pushed the dv2/mysql_raw_tables branch 2 times, most recently from 807fef1 to 19bb0fc Compare April 22, 2024 23:17
/*
* We want to generate a query like:
*
* LOAD DATA LOCAL INFILE '/a/b/c' INTO TABLE foo.bar FIELDS TERMINATED BY ',' ENCLOSED BY
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.... thanks, autoformat

@edgao edgao force-pushed the dv2/td_tests_allow_unsafe_cast_2 branch from ecffdd8 to 307c476 Compare April 23, 2024 21:23
@edgao edgao force-pushed the dv2/td_tests_allow_unsafe_cast_2 branch from 307c476 to 6f1f8b4 Compare April 23, 2024 22:22
@edgao edgao force-pushed the dv2/td_tests_allow_unsafe_cast_2 branch from 6f1f8b4 to 4be8c70 Compare April 24, 2024 16:28
@edgao edgao force-pushed the dv2/td_tests_allow_unsafe_cast_2 branch from 4be8c70 to 8e32610 Compare April 24, 2024 16:46
@edgao edgao force-pushed the dv2/mysql_raw_tables branch 2 times, most recently from bafc309 to 38c8d07 Compare April 24, 2024 17:43
@edgao edgao force-pushed the dv2/td_tests_allow_unsafe_cast_2 branch from 8e32610 to 1004c52 Compare April 24, 2024 17:57
@edgao edgao force-pushed the dv2/mysql_raw_tables branch 3 times, most recently from 3b4f4fc to 33935e7 Compare April 25, 2024 20:55
@edgao edgao force-pushed the dv2/td_tests_allow_unsafe_cast_2 branch from 1004c52 to 7ae481a Compare April 26, 2024 20:19
@edgao edgao force-pushed the dv2/td_tests_allow_unsafe_cast_2 branch from 7ae481a to 2067931 Compare April 26, 2024 22:58
@edgao edgao marked this pull request as ready for review April 26, 2024 23:57
@edgao edgao force-pushed the dv2/td_tests_allow_unsafe_cast_2 branch from 2067931 to be524ad Compare April 29, 2024 15:10
@edgao
Copy link
Contributor Author

edgao commented Apr 29, 2024

closing this PR in favor of #36936

@edgao edgao closed this Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants