🐛Destination-Snowflake: Added "Table/Stage already exists, but no permissions" handler on Sync action - #21912
Conversation
… no permissions" handler on Sync action
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.56 |
✅ | ✅ |
destination-rockset |
0.1.4 |
✅ | ✅ |
destination-snowflake |
0.4.47 |
✅ | ✅ |
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=connectors/destination-snowflake
Build PassedTest summary info: |
Airbyte Code Coverage
|
| @Override | ||
| public void createTableIfNotExists(final JdbcDatabase database, final String schemaName, final String tableName) throws SQLException { | ||
| database.execute(createTableQuery(database, schemaName, tableName)); | ||
| try{ |
There was a problem hiding this comment.
nit: add space before {
There was a problem hiding this comment.
updated, thanks
Greg Solovyev (grishick)
left a comment
There was a problem hiding this comment.
LGTM except for one missing space between try and {. Don't forget to publish a new version of Snowflake destination before merging.
|
Also, let's wait for this PR to merge (since it is already publishing): #20631 and then rebase this PR before publishing |
| ENV ENABLE_SENTRY true | ||
|
|
||
| LABEL io.airbyte.version=0.4.45 | ||
| LABEL io.airbyte.version=0.4.46 |
There was a problem hiding this comment.
this will change after this PR is merged: #20631
There was a problem hiding this comment.
yup, thanks. I'd bumped the version and after pushing noticed your comment to wait for another PR. It was 1 minute earlier :)
| Assertions.assertInstanceOf(ConfigErrorException.class, exception); | ||
| } else { | ||
| Assertions.assertInstanceOf(SnowflakeSQLException.class, exception); | ||
| Assertions.assertEquals(exception.getMessage(), message); |
There was a problem hiding this comment.
Minor nit: you probably don't need to explicitly define Assertions.assertEquals since at the top of the imports we import
import static org.junit.jupiter.api.Assertions.assertEquals;Likely you can also do the same thing with assertInstanceOf
| public void testCreateTableIfNotExists(final String message, final boolean shouldCapture) { | ||
| final JdbcDatabase db = Mockito.mock(JdbcDatabase.class); | ||
| final var schemaName = "foo"; | ||
| final var tableName = "bar"; |
There was a problem hiding this comment.
Minor nit: since we know these values in the createTableIfNotExists takes in String values to also just have the test reflect that. It shouldn't be necessary that the tests are using type inferences when the method defines the type explicitly
Ryan Fu (ryankfu)
left a comment
There was a problem hiding this comment.
Non-blocking nits but as Greg mentioned to wait on the publication and merging of the checkpointing changes in #20631. I can ping you once the publication is done although it might take some time since destination-snowflake takes close to 4 hours to complete the integration tests
…sts-handlers # Conflicts: # docs/integrations/destinations/snowflake.md
|
/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 |
What
Users may try to write to the same destination from different connectors. In this case, we may get a "Table/Stage already exists, but no permissions" error msg.
How
Added handlers to capture it and report it as a config error
🚨 User Impact 🚨
No breaking changes expected
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.