Write a temporary table to verify write permissions when checking destination JDBC credentials #1834
Conversation
|
/test connector=destination-bigquery
|
|
/test connector=destination-snowflake
|
|
/test connector=destination-postgres
|
|
/test connector=destination-redshift
|
...ion-jdbc/src/main/java/io/airbyte/integrations/destination/jdbc/AbstractJdbcDestination.java
Outdated
Show resolved
Hide resolved
…/io/airbyte/integrations/destination/jdbc/AbstractJdbcDestination.java
| String outputTableName = "_airbyte_connection_test_" + UUID.randomUUID().toString().replaceAll("-", ""); | ||
| sqlOperations.createSchemaIfNotExists(database, outputSchema); | ||
| sqlOperations.createTableIfNotExists(database, outputSchema, outputTableName); | ||
| sqlOperations.dropTableIfExists(database, outputSchema, outputTableName); |
There was a problem hiding this comment.
don't we need to drop the schema too?
There was a problem hiding this comment.
oh i guess sql operations doesn't have this.
There was a problem hiding this comment.
We wouldn't wnat to drop the schema if it already existed
There was a problem hiding this comment.
ignore this. i thought this was a test schema.
There was a problem hiding this comment.
i am really not a big fan of there being a side effect to a check connection. talked through this with @sherifnada, i'm convinced that there's not a great way of getting this information out of the metadata provided by jdbc. assuming that we don't want to write custom code for each db, this seems like the only way we can verify that the destination will be able to create tables in the target schema. i think i'm convinced that this is something worth checking at check connection and will generally make a user's life better.
What
Closes #1772
Enhance the
checkmethod for JDBC destinations by creating & dropping a temporary table. The "correct" way to do this should be to write DB-specific queries (e.g to check the right metadata tables) that don't incur any side-effects, but getting this in for the upcoming release, then we can revisit.Pre-merge checklist