check for permissions error on snowflake destinations #21764
Conversation
octavia-squidington-iv
added
area/connectors
Affected Connector ReportNOTE
|
| Connector | Version | Changelog | Publish |
|---|
- See "Actionable Items" below for how to resolve warnings and errors.
❌ Destinations (21)
| Connector | Version | Changelog | Publish |
|---|---|---|---|
destination-azure-blob-storage |
0.1.6 |
✅ | ✅ |
destination-clickhouse |
0.2.2 |
❌ (changelog missing) |
✅ |
destination-clickhouse-strict-encrypt |
0.2.2 |
🔵 (ignored) |
🔵 (ignored) |
destination-databricks |
0.3.1 |
✅ | ✅ |
destination-dynamodb |
0.1.7 |
✅ | ✅ |
destination-gcs |
0.2.14 |
✅ | ✅ |
destination-mariadb-columnstore |
0.1.7 |
✅ | ✅ |
destination-mssql |
0.1.22 |
✅ | ✅ |
destination-mssql-strict-encrypt |
0.1.22 |
🔵 (ignored) |
🔵 (ignored) |
destination-mysql |
0.1.20 |
✅ | ✅ |
destination-mysql-strict-encrypt |
❌ 0.1.21(mismatch: 0.1.20) |
🔵 (ignored) |
🔵 (ignored) |
destination-oracle |
0.1.19 |
✅ | ✅ |
destination-oracle-strict-encrypt |
0.1.19 |
🔵 (ignored) |
🔵 (ignored) |
destination-postgres |
0.3.26 |
✅ | ✅ |
destination-postgres-strict-encrypt |
0.3.26 |
🔵 (ignored) |
🔵 (ignored) |
destination-redshift |
0.3.54 |
✅ | ✅ |
destination-rockset |
0.1.4 |
✅ | ✅ |
destination-snowflake |
0.4.45 |
✅ | ✅ |
destination-teradata |
0.1.0 |
✅ | ✅ |
destination-tidb |
0.1.0 |
✅ | ✅ |
destination-yugabytedb |
0.1.0 |
✅ | ✅ |
- See "Actionable Items" below for how to resolve warnings and errors.
👀 Other Modules (1)
- base-normalization
Actionable Items
(click to expand)
| Category | Status | Actionable Item |
|---|---|---|
| Version | ❌ mismatch |
The version of the connector is different from its normal variant. Please bump the version of the connector. |
| ⚠ doc not found |
The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like source-jdbc is not published or documented). Please double-check to make sure that it is not a bug. |
|
| Changelog | ⚠ doc not found |
The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like source-jdbc is not published or documented). Please double-check to make sure that it is not a bug. |
| ❌ changelog missing |
There is no chnagelog for the current version of the connector. If you are the author of the current version, please add a changelog. | |
| Publish | ⚠ not in seed |
The connector is not in the seed file (e.g. source_definitions.yaml), so its publication status cannot be checked. This can be normal (e.g. some connectors are cloud-specific, and only listed in the cloud seed file). Please double-check to make sure that it is not a bug. |
| ❌ diff seed version |
The connector exists in the seed file, but the latest version is not listed there. This usually means that the latest version is not published. Please use the /publish command to publish the latest version. |
|
/test connector=destination-snowflake
Build FailedTest summary info: |
|
/test connector=connectors/destination-snowflake
Build FailedTest summary info: |
|
/test connector=connectors/destination-jdbc
Build PassedTest summary info: |
Airbyte Code Coverage
|
| * @param e the exception to check. | ||
| * @return A ConfigErrorException with a message with actionable feedback to the user. | ||
| */ | ||
| protected Optional<ConfigErrorException> checkForKnownConfigExceptions(Exception e) { |
There was a problem hiding this comment.
I like this! and 👍 for good javadoc
| final var schemaName = "foo"; | ||
| try { | ||
| Mockito.doThrow(new SQLException("TEST")).when(db).execute(Mockito.anyString()); | ||
| createSchemaIfNotExists(db, schemaName); |
There was a problem hiding this comment.
I think there's an assertThrows method for this exact situation - currently your test will pass even if this method doesn't throw
There was a problem hiding this comment.
Ahh good catch! that's what I was trying to remember
| @Override | ||
| protected Optional<ConfigErrorException> checkForKnownConfigExceptions(Exception e) { | ||
| if (e instanceof SnowflakeSQLException && e.getMessage().contains(NO_PRIVILEGES_ERROR_MESSAGE)) { | ||
| return Optional.of(new ConfigErrorException("Encountered Error with Snowflake Configuration, please verify your privileges", e)); |
There was a problem hiding this comment.
can we be more specific about what permissions are needed? (this is an actual question, I don't know how specific snowflake's error message is)
There was a problem hiding this comment.
+1 to Ed's comment, do we know what are the ways a customer can resolve this
Schema 'PUBLIC' already exists, but current role has no privileges on it. If this is unexpected and you cannot resolve this problem, contact your system administrator.
Also, looking at the SnowflakeSQLException class, it extends off of SQLException which has a method getSQLState(), is that something we can more accurately map so this test doesn't become fragile?
There was a problem hiding this comment.
The SQL state was SQLState = "42710" but AFAICT this didn't map to anything beside the SQLComilationError here's the entire content of the exception
this = {SnowflakeInternalStagingSqlOperations@5796}
e = {SnowflakeSQLException@5797} "net.snowflake.client.jdbc.SnowflakeSQLException: SQL compilation error:\nSchema 'PUBLIC' already exists, but current role has no privileges on it. If this is unexpected and you cannot resolve this problem, contact your system administrator. ACCOUNTADMIN role may be required to manage the privileges on the object."
queryId = "01a9da10-0000-eef5-0000-53b903d214ba"
retryCount = 0
issocketTimeoutNoBackoff = false
elapsedSeconds = 0
SQLState = "42710"
vendorCode = 3041
next = null
backtrace = {Object[6]@5801}
detailMessage = "SQL compilation error:\nSchema 'PUBLIC' already exists, but current role has no privileges on it. If this is unexpected and you cannot resolve this problem, contact your system administrator. ACCOUNTADMIN role may be required to manage the privileges on the object."
cause = {SnowflakeSQLException@5797} "net.snowflake.client.jdbc.SnowflakeSQLException: SQL compilation error:\nSchema 'PUBLIC' already exists, but current role has no privileges on it. If this is unexpected and you cannot resolve this problem, contact your system administrator. ACCOUNTADMIN role may be required to manage the privileges on the object."
stackTrace = {StackTraceElement[29]@5806}
depth = 29
suppressedExceptions = {Collections$EmptyList@5804} size = 0
I'm open to any thoughts here
There was a problem hiding this comment.
This is a little tricky since Snowflake has poor documentation on their SQLSTATE codes but it appears a few other DB documentation Postgres and IBM both use 42710 for marking duplicate object
I'll leave it up to your discretion, although I might consider if you go with a string check to have it
ACCOUNTADMIN role may be required to manage the privileges on the object.
Since if someone were to see that within the code it makes a bit more sense what they may need to do in order to resolve their issue. Granted the usage of the ConfigErrorException is for a actionable message for the user to resolve this configuration issue
| database.execute(String.format("CREATE SCHEMA IF NOT EXISTS %s;", schemaName)); | ||
| schemaSet.add(schemaName); | ||
| } | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
maybe we should log the original exception here, since it's hard to guarantee that subclasses always handle it correctly? I'm always a bit conflicted about this, since we might end up logging the same exception multiple times
There was a problem hiding this comment.
Since the use case hasn't presented itself I'm going to err on the side of inaction and not assume anything just yet. As for now the current behavior will log the original exception and the snowflake subclass will pass along the original exception in the Config Exception
|
@jbfbell base on this comment, it sounds like you followed all the steps that Airbyte documents for getting a user setup with a sync between Postgres <> Snowflake. Since it sounds like this change will only convert the Exception to a Were you able to find the steps to mitigate this issue and if so that seems we should update our |
@ryankfu I think its only an issue if the user chooses to "mirror source schema" Maybe this is something to address as a part of #20561 ? |
|
@jbfbell If I'm reading that ticket correctly, it's the true fix to the issue, however, if we're marking an |
octavia-squidington-iv
added
the
area/documentation
|
@ryankfu Added some troubleshooting docs |
|
/publish connector=connectors/destination-snowflake
if you have connectors that successfully published but failed definition generation, follow step 4 here |
| If you receive an error stating `Current role does not have permissions on the target schema` make sure that the | ||
| Snowflake destination `SCHEMA` is one that the role you've provided has permissions on. When creating a connection, | ||
| it may allow you to select `Mirror source structure` for the `Destination namespace`, which if you have followed | ||
| some of our default examples and tutorials may result in the connection trying to right to a `PUBLIC` schema. |
There was a problem hiding this comment.
I think this should be write instead of right
|
/test connector=connectors/destination-snowflake
Build FailedTest summary info: |
|
/test connector=connectors/destination-snowflake
Build PassedTest summary info: |
|
/publish connector=connectors/destination-snowflake
if you have connectors that successfully published but failed definition generation, follow step 4 here |
octavia-squidington-iii
temporarily deployed
to
more-secrets
GitHub Actions
Inactive
octavia-squidington-iii
temporarily deployed
to
more-secrets
GitHub Actions
Inactive
What
Check for known Configuration Issues with the Snowflake Destination for #20343
How
Verify the exceptions thrown do not match any of our expected exception scenarios.
🚨 User Impact 🚨
For users who walk through the Snowflake Destination setup and select "Mirror Source" ( or any user who doesn't have the permissions properly configured) they will see a configuration error in the UI.
Pre-merge Checklist
Expand the relevant checklist and delete the others.
New Connector
Community member or Airbyter
airbyte_secret./gradlew :airbyte-integrations:connectors:<name>:integrationTest.README.mdbootstrap.md. See description and examplesdocs/integrations/<source or destination>/<name>.mdincluding changelog. See changelog exampledocs/integrations/README.mdairbyte-integrations/builds.mdAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>command is passing/publishcommand described hereUpdating a connector
Community member or Airbyter
airbyte_secret./gradlew :airbyte-integrations:connectors:<name>:integrationTest.README.mdbootstrap.md. See description and examplesdocs/integrations/<source or destination>/<name>.mdincluding changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>command is passing/publishcommand described hereConnector Generator
-scaffoldin their name) have been updated with the latest scaffold by running./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplatesthen checking in your changesTests
Unit
Put your unit tests output here.
Integration
Put your integration tests output here.
Acceptance
Put your acceptance tests output here.