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.

Sign up for GitHub

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

Jump to bottom

Migrate Postgres and MySql to use new JdbcSource #1307

Merged
merged 11 commits into from
Jan 8, 2021
Merged

Migrate Postgres and MySql to use new JdbcSource #1307

merged 11 commits into from
Jan 8, 2021

Conversation

cgardens
Copy link
Contributor

@cgardens cgardens commented Dec 13, 2020 •
edited
Loading

What

  • Migrate Postgres and MySql Sources to use JdbcSource instead of JooqSource.
  • This will make them safe from OOM (current JOOQ implementation is not safe).
  • Adds the ability to test each of them with more data than can fit in memory to guarantee they are in fact streaming results.

@cgardens cgardens changed the base branch from master to cgardens/jdbc_source_incremental4 December 13, 2020 21:06
@cgardens cgardens force-pushed the cgardens/jdbc_source_incremental4 branch 4 times, most recently from 0d400d7 to 7d48ef4 Compare December 18, 2020 22:09
Base automatically changed from cgardens/jdbc_source_incremental4 to master December 18, 2020 22:17
@cgardens cgardens force-pushed the cgardens/jdbc_source_incremental5 branch from 19e6f62 to 2325e8f Compare December 23, 2020 01:46
@cgardens cgardens marked this pull request as ready for review December 23, 2020 01:54
@cgardens cgardens changed the title wip migrate pg and mysql to abstract jdbc source, add datetime tests Migrate Postgres and MySql to use new JdbcSource Dec 23, 2020
michel-tricot
michel-tricot approved these changes Dec 23, 2020
View reviewed changes
Copy link
Contributor

@michel-tricot michel-tricot left a comment

Choose a reason for hiding this comment

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

Tada!

...urce-mysql/src/test/java/io/airbyte/integrations/source/mysql/MySqlDbSourceStandardTest.java Outdated
import org.junit.jupiter.api.BeforeEach;
import org.testcontainers.containers.MySQLContainer;

class MySqlDbSourceStandardTest extends JdbcSourceStandardTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: remove the db from the name

...gres/src/test/java/io/airbyte/integrations/source/postgres/PostgresDbSourceStandardTest.java Outdated
import org.testcontainers.containers.PostgreSQLContainer;
import org.testcontainers.utility.MountableFile;

class PostgresDbSourceStandardTest extends JdbcSourceStandardTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same NIT

...stgres/src/test/java/io/airbyte/integrations/source/postgres/PostgresJdbcJdbcStressTest.java Outdated
* in source-postgres.
*/
@Disabled
class PostgresJdbcJdbcStressTest extends JdbcStressTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

You can can remove one of the JDBC :)

@cgardens cgardens force-pushed the cgardens/jdbc_source_incremental5 branch from 8c487d9 to 3d490fc Compare January 4, 2021 15:38
@cgardens cgardens merged commit 102b432 into master Jan 8, 2021
@cgardens cgardens deleted the cgardens/jdbc_source_incremental5 branch January 8, 2021 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michel-tricot michel-tricot michel-tricot approved these changes

@sherifnada sherifnada Awaiting requested review from sherifnada

@jrhizor jrhizor Awaiting requested review from jrhizor

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cgardens @michel-tricot