Skip to content

CDK: fixes for destination-postgres #36619

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
Merged
merged 1 commit into from
Mar 29, 2024
Merged

CDK: fixes for destination-postgres #36619

merged 1 commit into from
Mar 29, 2024

Conversation

gisripa
Copy link
Contributor

@gisripa gisripa commented Mar 28, 2024

No description provided.

@vercel Vercel
Copy link

vercel bot commented Mar 28, 2024 •
edited
Loading

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 Mar 29, 2024 5:32pm

@octavia-squidington-iii octavia-squidington-iii added the CDK Connector Development Kit label Mar 28, 2024
This was referenced Mar 28, 2024
Merged
Merged
@gisripa Graphite App
Copy link
Contributor Author

gisripa commented Mar 28, 2024 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @gisripa and the rest of your teammates on Graphite Graphite

@gisripa gisripa mentioned this pull request Mar 28, 2024
Merged
@gisripa gisripa changed the title cdkk-dest-postgres-fixes CDK: fixes for destination-postgres Mar 28, 2024
@gisripa gisripa marked this pull request as ready for review March 28, 2024 20:55
@gisripa gisripa requested review from a team March 28, 2024 20:55
@gisripa gisripa force-pushed the gireesh/cdkk-destination-fixes branch from 08d1dcd to 9370fc8 Compare March 28, 2024 20:57
@gisripa gisripa force-pushed the gireesh/03-28-cdkk-dest-postgres-fixes branch 2 times, most recently from d88ed75 to 27e139d Compare March 28, 2024 21:20
gisripa
gisripa commented Mar 28, 2024
View reviewed changes
airbyte-cdk/java/airbyte-cdk/core/src/main/kotlin/io/airbyte/cdk/db/jdbc/JdbcUtils.kt
@@ -62,7 +62,7 @@ object JdbcUtils {
JDBCType.VARCHAR,
JDBCType.LONGVARCHAR
)
@JvmField val defaultSourceOperations: JdbcSourceOperations = JdbcSourceOperations()
@JvmStatic val defaultSourceOperations: JdbcSourceOperations = JdbcSourceOperations()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jdpgrailsdev This was accessed as a static member in Java, any better way to do than the escape hatch ?

Copy link
Contributor

Choose a reason for hiding this comment

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

not sure what the escape hatch is

@gisripa gisripa force-pushed the gireesh/cdkk-destination-fixes branch from 9370fc8 to cd4a7c2 Compare March 28, 2024 22:41
stephane-airbyte
stephane-airbyte reviewed Mar 28, 2024
View reviewed changes
...airbyte-cdk/core/src/main/kotlin/io/airbyte/cdk/integrations/base/AirbyteExceptionHandler.kt
@@ -154,8 +154,7 @@ class AirbyteExceptionHandler : Thread.UncaughtExceptionHandler {
}
}

@VisibleForTesting
fun addCommonStringsToDeinterpolate() {
internal fun addCommonStringsToDeinterpolate() {
Copy link
Contributor

Choose a reason for hiding this comment

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

yes

stephane-airbyte
stephane-airbyte reviewed Mar 28, 2024
View reviewed changes
...e-cdk/java/airbyte-cdk/core/src/testFixtures/kotlin/io/airbyte/cdk/testutils/TestDatabase.kt
}

companion object {
val DEFAULT_CDC_REPLICATION_INITIAL_WAIT: Duration? = Duration.ofSeconds(5)
private val DEFAULT_CDC_REPLICATION_INITIAL_WAIT: Duration = Duration.ofSeconds(5)
Copy link
Contributor

@stephane-airbyte stephane-airbyte Mar 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@JvmStatic protected val DEFXXX: Duration = XXX

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in java it looks like this getDEFAULT_CDC_REPLICATION_INITIAL_WAIT() by using above convention. Not sure why. its part of nested ConfigBuilder class's companion not the TestDatabase

Copy link
Contributor

@stephane-airbyte stephane-airbyte Mar 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

what about @JvmField? That should get rid of the getter, no?

@gisripa gisripa force-pushed the gireesh/cdkk-destination-fixes branch from cd4a7c2 to f302124 Compare March 28, 2024 23:46
@gisripa gisripa force-pushed the gireesh/03-28-cdkk-dest-postgres-fixes branch 2 times, most recently from 95fc6d8 to 860e413 Compare March 29, 2024 00:12
@octavia-squidington-iv octavia-squidington-iv requested review from a team March 29, 2024 00:27
Base automatically changed from gireesh/cdkk-destination-fixes to master March 29, 2024 17:08
@gisripa gisripa force-pushed the gireesh/03-28-cdkk-dest-postgres-fixes branch 2 times, most recently from df6c83e to 6871baa Compare March 29, 2024 17:24
@gisripa gisripa force-pushed the gireesh/03-28-cdkk-dest-postgres-fixes branch from 6871baa to dada46a Compare March 29, 2024 17:32
@gisripa
Copy link
Contributor Author

gisripa commented Mar 29, 2024 •
edited by github-actions bot
Loading

/publish-java-cdk

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

@gisripa gisripa merged commit 4b85dba into master Mar 29, 2024
27 checks passed
@gisripa gisripa deleted the gireesh/03-28-cdkk-dest-postgres-fixes branch March 29, 2024 17:55
nurikk pushed a commit to nurikk/airbyte that referenced this pull request Apr 4, 2024
@gisripa @nurikk-sa
CDK: fixes for destination-postgres (airbytehq#36619)
9c04824
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stephane-airbyte stephane-airbyte stephane-airbyte approved these changes

Assignees
No one assigned
Labels
CDK Connector Development Kit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gisripa @stephane-airbyte @octavia-squidington-iii