[Source-postgres] : Move to new Kotlin CDK #36584
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
octavia-squidington-iii
added
area/connectors
| @@ -318,7 +318,7 @@ public JdbcDatabase createDatabase(final JsonNode sourceConfig) throws SQLExcept | |||
| driverClassName, | |||
| jdbcConfig.get(JdbcUtils.JDBC_URL_KEY).asText(), | |||
| connectionProperties, | |||
| getConnectionTimeout(connectionProperties, driverClassName)); | |||
There was a problem hiding this comment.
This change concerns me a little bit. There's some special logic for timeouts WRT postgres, mysql or sqlserver. I think we should move getConnectionTimeout(Map<String, String>, String? out of the companion object instead of using the method that doesn't take a driverClassName
There was a problem hiding this comment.
I marked the function getConnectionTimeout(Map<String, String>, String? in the companion object in JdbcConnector as @JvmStatic. I'm not really sure why this class exists in the whole hierarchy. Feels like it can be guttedbut this seems to be the most innocuous change IMO
There was a problem hiding this comment.
that works. Thank you.
The class should eventually be in the hierarchy, and the DB specific behavior should be decided by each connector, rather than being coded inside the CDK, which means it'll eventually move back to being non-static
...ce-postgres/src/test/java/io/airbyte/integrations/source/postgres/CdcPostgresSourceTest.java
Outdated
Show resolved
Hide resolved
| @@ -234,14 +232,6 @@ public boolean supportsSchemas() { | |||
| return true; | |||
| } | |||
|
|
|||
| @Test | |||
| void testSpec() throws Exception { | |||
There was a problem hiding this comment.
There's a function in the parent class which is pretty much doing the same exact thing : https://github.com/airbytehq/airbyte/blob/master/airbyte-cdk/java/airbyte-cdk/db-sources/src/testFixtures/kotlin/io/airbyte/cdk/integrations/source/jdbc/test/JdbcSourceAcceptanceTest.kt#L193
octavia-squidington-iii
added
the
area/documentation
|
/publish-java-cdk
|
No description provided.