From 9d6fe68d6fd0628ec6d865acf59680325cc8bb57 Mon Sep 17 00:00:00 2001 From: Eugene Date: Mon, 30 Jan 2023 19:33:39 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9BDestination-Snowflake:=20Added=20"T?= =?UTF-8?q?able/Stage=20already=20exists,=20but=20no=20permissions"=20hand?= =?UTF-8?q?ler=20on=20Sync=20action=20(#21912)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [21843] Destination-Snowflake: Added "Table/Stage already exists, but no permissions" handler on Sync action --------- Co-authored-by: Octavia Squidington III --- .../seed/destination_definitions.yaml | 2 +- .../resources/seed/destination_specs.yaml | 2 +- .../destination/jdbc/JdbcSqlOperations.java | 6 +++- .../destination-snowflake/Dockerfile | 2 +- ...SnowflakeInternalStagingSqlOperations.java | 6 +++- .../snowflake/SnowflakeSqlOperations.java | 4 ++- ...flakeInternalStagingSqlOperationsTest.java | 29 ++++++++++++++++ .../snowflake/SnowflakeSqlOperationsTest.java | 33 ++++++++++++++++--- docs/integrations/destinations/snowflake.md | 1 + 9 files changed, 74 insertions(+), 11 deletions(-) diff --git a/airbyte-config/init/src/main/resources/seed/destination_definitions.yaml b/airbyte-config/init/src/main/resources/seed/destination_definitions.yaml index 01bc2e8eb9dc7..002404443f58d 100644 --- a/airbyte-config/init/src/main/resources/seed/destination_definitions.yaml +++ b/airbyte-config/init/src/main/resources/seed/destination_definitions.yaml @@ -348,7 +348,7 @@ - name: Snowflake destinationDefinitionId: 424892c4-daac-4491-b35d-c6688ba547ba dockerRepository: airbyte/destination-snowflake - dockerImageTag: 0.4.46 + dockerImageTag: 0.4.47 documentationUrl: https://docs.airbyte.com/integrations/destinations/snowflake icon: snowflake.svg normalizationConfig: diff --git a/airbyte-config/init/src/main/resources/seed/destination_specs.yaml b/airbyte-config/init/src/main/resources/seed/destination_specs.yaml index a383f60365bd2..94ddbba34d970 100644 --- a/airbyte-config/init/src/main/resources/seed/destination_specs.yaml +++ b/airbyte-config/init/src/main/resources/seed/destination_specs.yaml @@ -6109,7 +6109,7 @@ supported_destination_sync_modes: - "overwrite" - "append" -- dockerImage: "airbyte/destination-snowflake:0.4.46" +- dockerImage: "airbyte/destination-snowflake:0.4.47" spec: documentationUrl: "https://docs.airbyte.com/integrations/destinations/snowflake" connectionSpecification: diff --git a/airbyte-integrations/connectors/destination-jdbc/src/main/java/io/airbyte/integrations/destination/jdbc/JdbcSqlOperations.java b/airbyte-integrations/connectors/destination-jdbc/src/main/java/io/airbyte/integrations/destination/jdbc/JdbcSqlOperations.java index 627e0def2f447..4af0184a7ed80 100644 --- a/airbyte-integrations/connectors/destination-jdbc/src/main/java/io/airbyte/integrations/destination/jdbc/JdbcSqlOperations.java +++ b/airbyte-integrations/connectors/destination-jdbc/src/main/java/io/airbyte/integrations/destination/jdbc/JdbcSqlOperations.java @@ -67,7 +67,11 @@ protected Optional checkForKnownConfigExceptions(Exception @Override public void createTableIfNotExists(final JdbcDatabase database, final String schemaName, final String tableName) throws SQLException { - database.execute(createTableQuery(database, schemaName, tableName)); + try { + database.execute(createTableQuery(database, schemaName, tableName)); + } catch (SQLException e) { + throw checkForKnownConfigExceptions(e).orElseThrow(() -> e); + } } @Override diff --git a/airbyte-integrations/connectors/destination-snowflake/Dockerfile b/airbyte-integrations/connectors/destination-snowflake/Dockerfile index f3b9f0849cb43..53c560baaab26 100644 --- a/airbyte-integrations/connectors/destination-snowflake/Dockerfile +++ b/airbyte-integrations/connectors/destination-snowflake/Dockerfile @@ -20,5 +20,5 @@ RUN tar xf ${APPLICATION}.tar --strip-components=1 ENV ENABLE_SENTRY true -LABEL io.airbyte.version=0.4.46 +LABEL io.airbyte.version=0.4.47 LABEL io.airbyte.name=airbyte/destination-snowflake diff --git a/airbyte-integrations/connectors/destination-snowflake/src/main/java/io/airbyte/integrations/destination/snowflake/SnowflakeInternalStagingSqlOperations.java b/airbyte-integrations/connectors/destination-snowflake/src/main/java/io/airbyte/integrations/destination/snowflake/SnowflakeInternalStagingSqlOperations.java index a0a521c54b305..5b555a36e03fb 100644 --- a/airbyte-integrations/connectors/destination-snowflake/src/main/java/io/airbyte/integrations/destination/snowflake/SnowflakeInternalStagingSqlOperations.java +++ b/airbyte-integrations/connectors/destination-snowflake/src/main/java/io/airbyte/integrations/destination/snowflake/SnowflakeInternalStagingSqlOperations.java @@ -130,7 +130,11 @@ protected String getListQuery(final String stageName, final String stagingPath, public void createStageIfNotExists(final JdbcDatabase database, final String stageName) throws Exception { final String query = getCreateStageQuery(stageName); LOGGER.debug("Executing query: {}", query); - database.execute(query); + try { + database.execute(query); + } catch (Exception e) { + throw checkForKnownConfigExceptions(e).orElseThrow(() -> e); + } } /** diff --git a/airbyte-integrations/connectors/destination-snowflake/src/main/java/io/airbyte/integrations/destination/snowflake/SnowflakeSqlOperations.java b/airbyte-integrations/connectors/destination-snowflake/src/main/java/io/airbyte/integrations/destination/snowflake/SnowflakeSqlOperations.java index 177227c3750d6..f414697775f5f 100644 --- a/airbyte-integrations/connectors/destination-snowflake/src/main/java/io/airbyte/integrations/destination/snowflake/SnowflakeSqlOperations.java +++ b/airbyte-integrations/connectors/destination-snowflake/src/main/java/io/airbyte/integrations/destination/snowflake/SnowflakeSqlOperations.java @@ -83,7 +83,9 @@ protected String generateFilesList(final List files) { @Override protected Optional checkForKnownConfigExceptions(Exception e) { if (e instanceof SnowflakeSQLException && e.getMessage().contains(NO_PRIVILEGES_ERROR_MESSAGE)) { - return Optional.of(new ConfigErrorException("Encountered Error with Snowflake Configuration: Current role does not have permissions on the target schema please verify your privileges", e)); + return Optional.of(new ConfigErrorException( + "Encountered Error with Snowflake Configuration: Current role does not have permissions on the target schema please verify your privileges", + e)); } return Optional.empty(); } diff --git a/airbyte-integrations/connectors/destination-snowflake/src/test/java/io/airbyte/integrations/destination/snowflake/SnowflakeInternalStagingSqlOperationsTest.java b/airbyte-integrations/connectors/destination-snowflake/src/test/java/io/airbyte/integrations/destination/snowflake/SnowflakeInternalStagingSqlOperationsTest.java index 77385c67fce0d..6038e86b401cc 100644 --- a/airbyte-integrations/connectors/destination-snowflake/src/test/java/io/airbyte/integrations/destination/snowflake/SnowflakeInternalStagingSqlOperationsTest.java +++ b/airbyte-integrations/connectors/destination-snowflake/src/test/java/io/airbyte/integrations/destination/snowflake/SnowflakeInternalStagingSqlOperationsTest.java @@ -5,10 +5,19 @@ package io.airbyte.integrations.destination.snowflake; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertTrue; +import io.airbyte.commons.exceptions.ConfigErrorException; +import io.airbyte.db.jdbc.JdbcDatabase; +import java.sql.SQLException; import java.util.List; +import net.snowflake.client.jdbc.SnowflakeSQLException; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.mockito.Mockito; class SnowflakeInternalStagingSqlOperationsTest { @@ -66,4 +75,24 @@ void removeStage() { assertEquals(expectedQuery, actualRemoveQuery); } + @ParameterizedTest + @CsvSource({"TEST,false", "but current role has no privileges on it,true"}) + public void testCreateStageIfNotExists(final String message, final boolean shouldCapture) { + final JdbcDatabase db = Mockito.mock(JdbcDatabase.class); + final String stageName = "foo"; + try { + Mockito.doThrow(new SnowflakeSQLException(message)).when(db).execute(Mockito.anyString()); + } catch (SQLException e) { + // This would not be expected, but the `execute` method above will flag as an unhandled exception + assert false; + } + final Exception exception = Assertions.assertThrows(Exception.class, () -> snowflakeStagingSqlOperations.createStageIfNotExists(db, stageName)); + if (shouldCapture) { + assertInstanceOf(ConfigErrorException.class, exception); + } else { + assertInstanceOf(SnowflakeSQLException.class, exception); + assertEquals(exception.getMessage(), message); + } + } + } diff --git a/airbyte-integrations/connectors/destination-snowflake/src/test/java/io/airbyte/integrations/destination/snowflake/SnowflakeSqlOperationsTest.java b/airbyte-integrations/connectors/destination-snowflake/src/test/java/io/airbyte/integrations/destination/snowflake/SnowflakeSqlOperationsTest.java index b52218e313545..ce86c3c109ae2 100644 --- a/airbyte-integrations/connectors/destination-snowflake/src/test/java/io/airbyte/integrations/destination/snowflake/SnowflakeSqlOperationsTest.java +++ b/airbyte-integrations/connectors/destination-snowflake/src/test/java/io/airbyte/integrations/destination/snowflake/SnowflakeSqlOperationsTest.java @@ -5,6 +5,8 @@ package io.airbyte.integrations.destination.snowflake; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; @@ -20,7 +22,6 @@ import java.util.ArrayList; import java.util.List; import net.snowflake.client.jdbc.SnowflakeSQLException; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; @@ -70,12 +71,34 @@ public void testCreateSchemaIfNotExists(final String message, final boolean shou // This would not be expected, but the `execute` method above will flag as an unhandled exception assert false; } - Exception exception = Assertions.assertThrows(Exception.class, () -> snowflakeSqlOperations.createSchemaIfNotExists(db, schemaName)); + Exception exception = assertThrows(Exception.class, () -> snowflakeSqlOperations.createSchemaIfNotExists(db, schemaName)); if (shouldCapture) { - Assertions.assertInstanceOf(ConfigErrorException.class, exception); + assertInstanceOf(ConfigErrorException.class, exception); } else { - Assertions.assertInstanceOf(SnowflakeSQLException.class, exception); - Assertions.assertEquals(exception.getMessage(), message); + assertInstanceOf(SnowflakeSQLException.class, exception); + assertEquals(exception.getMessage(), message); + } + } + + @ParameterizedTest + @CsvSource({"TEST,false", "but current role has no privileges on it,true"}) + public void testCreateTableIfNotExists(final String message, final boolean shouldCapture) { + final JdbcDatabase db = Mockito.mock(JdbcDatabase.class); + final String schemaName = "foo"; + final String tableName = "bar"; + try { + Mockito.doThrow(new SnowflakeSQLException(message)).when(db).execute(Mockito.anyString()); + } catch (SQLException e) { + // This would not be expected, but the `execute` method above will flag as an unhandled exception + assert false; + } + final Exception exception = + assertThrows(Exception.class, () -> snowflakeSqlOperations.createTableIfNotExists(db, schemaName, tableName)); + if (shouldCapture) { + assertInstanceOf(ConfigErrorException.class, exception); + } else { + assertInstanceOf(SnowflakeSQLException.class, exception); + assertEquals(exception.getMessage(), message); } } diff --git a/docs/integrations/destinations/snowflake.md b/docs/integrations/destinations/snowflake.md index f856956530e3b..0eb592a9e6f9d 100644 --- a/docs/integrations/destinations/snowflake.md +++ b/docs/integrations/destinations/snowflake.md @@ -287,6 +287,7 @@ Otherwise, make sure to grant the role the required permissions in the desired n | Version | Date | Pull Request | Subject | |:--------|:-----------|:-----------------------------------------------------------|:----------------------------------------------------------------------------------------------------------------------------------------------------| +| 0.4.47 | 2023-01-30 | [\#21912](https://github.com/airbytehq/airbyte/pull/21912) | Catch "Create" Table and Stage Known Permissions and rethrow as ConfigExceptions | | 0.4.46 | 2023-01-26 | [\#20631](https://github.com/airbytehq/airbyte/pull/20631) | Added support for destination checkpointing with staging | | 0.4.45 | 2023-01-25 | [#21087](https://github.com/airbytehq/airbyte/pull/21764) | Catch Known Permissions and rethrow as ConfigExceptions | | 0.4.44 | 2023-01-20 | [#21087](https://github.com/airbytehq/airbyte/pull/21087) | Wrap Authentication Errors as Config Exceptions |