fix: database tests and introduce a new parameter #2981

sfc-gh-jcieslak · 2024-08-06T12:55:02Z

Changes

[Bug]: snowflake_database with the from_database won't migrate automatically for 0.93 #2978 (adjust migration guide)
Fix database tests (remove tests for state upgraders, because they were incorrect and document step-by-step process for manual testing; I went through all of it)
Introduce a new parameter to address [Bug]: Cannot configure PUBLIC schema of new database #2826 and test it
- added a drop with retries just to be sure, but I don't think it should ever fail other than some kind of strange networking error between database create and drop schema
- I opted to drop the schema after setting id, so if we manage to create a database, but not drop the schema, the provider will print out a warning message that the public schema couldn't be dropped and it should be done manually. The other approaches I considered:
  - Do it before setting ID, but that would mean the Terraform running create again and failing on CREATE DATABASE (because it's already there).
  - Do it like right now, but make sure the public schema is gone in the alter, but I didn't go for that, because it could lead to further complications with (what is considered "after create"; after successful create? how would we keep the state of create status? etc.).

pkg/resources/database.go

pkg/resources/database_acceptance_test.go

Co-authored-by: Jakub Michalak <jakub.michalak@snowflake.com>

sfc-gh-asawicki · 2024-08-06T13:36:10Z

pkg/resources/database.go

@@ -22,6 +25,11 @@ var databaseSchema = map[string]*schema.Schema{
 		Required:    true,
 		Description: "Specifies the identifier for the database; must be unique for your account. As a best practice for [Database Replication and Failover](https://docs.snowflake.com/en/user-guide/db-replication-intro), it is recommended to give each secondary database the same name as its primary database. This practice supports referencing fully-qualified objects (i.e. '<db>.<schema>.<object>') by other objects in the same database, such as querying a fully-qualified table name in a view. If a secondary database has a different name from the primary database, then these object references would break in the secondary database.",
 	},
+	"drop_public_schema_on_creation": {


let's use DiffSuppressFunc: IgnoreAfterCreation,

Yup, already used it after Kuba's comment

sfc-gh-asawicki · 2024-08-06T13:39:46Z

pkg/resources/database_acceptance_test.go

-	name = "%s"
-	data_retention_time_in_days = 0 # to avoid in-place update to -1
-	from_replica = %s
+func doesNotContainPublicSchema(t *testing.T, id sdk.AccountObjectIdentifier) resource.TestCheckFunc {


We are moving away from these assertions and move to the new ones. I just say it to remind you this. Please keep that in mind in other PRs.

Here, please at least move these two funcs to pkg/acceptance/snowflakechecks/database.go so that we have all the ones that will be replaced in one place

Yeah, I tried to use the new ones, but here I'm asserting describe output that's not supported in the new assertions (also checked and it's documented in "known limitations"). Moved functions to pkg/acceptance/snowflakechecks/database.go as requested.

sfc-gh-asawicki · 2024-08-06T13:41:37Z

pkg/resources/database_acceptance_test.go

-resource "snowflake_secondary_database" "test" {
-	name = "%s"
-	as_replica_of = %s
+/*


Thanks for addding the manual steps that currently can't be run using this testing framework.

I think that we should no have the directly as comment in the acceptance test but maybe rather let's add manual tests directory (or something similar), let's add a short readme why we do this, and there let's add them as separate files.

WDYT @sfc-gh-jcieslak @sfc-gh-jmichalak ?

Good idea, maybe some of them could be automatic after upgrading to plugin framework.

github-actions · 2024-08-06T13:52:00Z

Integration tests failure for a1444783981a4faae6ede35289ffb64b7565e621

github-actions · 2024-08-06T14:09:15Z

Integration tests failure for be1f9b8ad89c781e61c6fd1d316314f7c2407da3

github-actions · 2024-08-06T14:34:53Z

Integration tests failure for 8e88ce022d660c4e9e615a2577330c57ea6d2df7

github-actions · 2024-08-07T15:00:00Z

Integration tests failure for 9f4dc3f1184c85d7ea00543eacf50ebf2f4aaffe

🤖 I have created a release *beep* *boop* --- ## [0.95.0](v0.94.1...v0.95.0) (2024-09-04) ### 🎉 **What's new:** * Add change_tracking, row access policy and aggregation policy to views ([#2988](#2988)) ([1f88bb1](1f88bb1)) * Add fully_qualified_name to all resources ([#2990](#2990)) ([1b0462f](1b0462f)) * Add identifier parsers ([#2957](#2957)) ([824ec52](824ec52)) * Add identifier with arguments ([#2979](#2979)) ([00ae1c5](00ae1c5)) * Add timeouts block to cortex ([#3004](#3004)) ([34d764b](34d764b)) * Add user parameters to resource ([#2968](#2968)) ([f4ae380](f4ae380)) * Conclude user rework ([#3036](#3036)) ([23e4625](23e4625)) * database role v1 readiness ([#3014](#3014)) ([c4db255](c4db255)) * Identifier with arguments for procedure and external function ([#2987](#2987)) ([f13cc5c](f13cc5c)) * Rework user resource ([#3026](#3026)) ([bde2638](bde2638)), closes [#1572](#1572) * Rework users datasource ([#3030](#3030)) ([751239b](751239b)), closes [#2902](#2902) * Upgrade view sdk ([#2969](#2969)) ([ef2d50a](ef2d50a)) * View rework part 2 ([#3021](#3021)) ([e05377d](e05377d)) * View rework part 3 ([#3023](#3023)) ([195b41c](195b41c)) ### 🔧 **Misc** * Add annotation about fully_qualified_name and fix handling granteeName ([#3009](#3009)) ([94e6345](94e6345)) * Apply identifier conventions ([#2996](#2996)) ([5cbea84](5cbea84)) * apply identifier conventions to grants ([#3008](#3008)) ([d7780ae](d7780ae)) * Clean collection utils ([#3028](#3028)) ([426ddb1](426ddb1)) * Clean old assertions ([#3029](#3029)) ([ad657eb](ad657eb)) * Conclude identifiers rework ([#3011](#3011)) ([c1b53f3](c1b53f3)) * Improve user test and add manual test for user default database and role ([#3035](#3035)) ([6cb0b4e](6cb0b4e)) * Use new identifier with arguments in function, external function and procedure grants ([#3002](#3002)) ([5053f8b](5053f8b)) * User improvements ([#3034](#3034)) ([65b64d7](65b64d7)) ### 🐛 **Bug fixes:** * database tests and introduce a new parameter ([#2981](#2981)) ([3bae7f6](3bae7f6)) * Fix custom diffs for fields with diff supression ([#3032](#3032)) ([2499602](2499602)) * Fix default secondary roles after BCR 2024_07 ([#3040](#3040)) ([2ca465a](2ca465a)), closes [#3038](#3038) * Fix issues 2972 and 3007 ([#3020](#3020)) ([1772387](1772387)) * Fix known user resource issues ([#3013](#3013)) ([a5dfeac](a5dfeac)) * identifier issues ([#2998](#2998)) ([6fb76b7](6fb76b7)) * minor issues ([#3027](#3027)) ([467b06e](467b06e)), closes [#3015](#3015) [#2807](#2807) [#3025](#3025) * Nuke users ([#2971](#2971)) ([0d90cc9](0d90cc9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: snowflake-release-please[bot] <105954990+snowflake-release-please[bot]@users.noreply.github.com>

fix database tests and introduce new parameter

a144478

sfc-gh-jmichalak reviewed Aug 6, 2024

View reviewed changes

pkg/resources/database.go Outdated Show resolved Hide resolved

pkg/resources/database_acceptance_test.go Outdated Show resolved Hide resolved

pkg/resources/database_acceptance_test.go Outdated Show resolved Hide resolved

sfc-gh-jcieslak and others added 2 commits August 6, 2024 15:35

Update pkg/resources/database_acceptance_test.go

be1f9b8

Co-authored-by: Jakub Michalak <jakub.michalak@snowflake.com>

changes after review

8e88ce0

sfc-gh-asawicki requested changes Aug 6, 2024

View reviewed changes

sfc-gh-jmichalak previously approved these changes Aug 6, 2024

View reviewed changes

changes after review

9f4dc3f

sfc-gh-jcieslak dismissed sfc-gh-jmichalak’s stale review via 9f4dc3f August 7, 2024 14:03

sfc-gh-jmichalak approved these changes Aug 7, 2024

View reviewed changes

sfc-gh-jcieslak merged commit 3bae7f6 into main Aug 8, 2024
8 of 9 checks passed

sfc-gh-jcieslak deleted the fix-database-tests branch August 8, 2024 09:01

snowflake-release-please bot mentioned this pull request Aug 6, 2024

chore(main): release 0.95.0 #2977

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: database tests and introduce a new parameter #2981

fix: database tests and introduce a new parameter #2981

sfc-gh-jcieslak commented Aug 6, 2024

sfc-gh-asawicki Aug 6, 2024

sfc-gh-jcieslak Aug 7, 2024 •

edited

Loading

sfc-gh-asawicki Aug 6, 2024

sfc-gh-jcieslak Aug 7, 2024 •

edited

Loading

sfc-gh-asawicki Aug 6, 2024

sfc-gh-jmichalak Aug 6, 2024

sfc-gh-jcieslak Aug 8, 2024

github-actions bot commented Aug 6, 2024

github-actions bot commented Aug 6, 2024

github-actions bot commented Aug 6, 2024

github-actions bot commented Aug 7, 2024

fix: database tests and introduce a new parameter #2981

fix: database tests and introduce a new parameter #2981

Conversation

sfc-gh-jcieslak commented Aug 6, 2024

Changes

sfc-gh-asawicki Aug 6, 2024

Choose a reason for hiding this comment

sfc-gh-jcieslak Aug 7, 2024 • edited Loading

Choose a reason for hiding this comment

sfc-gh-asawicki Aug 6, 2024

Choose a reason for hiding this comment

sfc-gh-jcieslak Aug 7, 2024 • edited Loading

Choose a reason for hiding this comment

sfc-gh-asawicki Aug 6, 2024

Choose a reason for hiding this comment

sfc-gh-jmichalak Aug 6, 2024

Choose a reason for hiding this comment

sfc-gh-jcieslak Aug 8, 2024

Choose a reason for hiding this comment

github-actions bot commented Aug 6, 2024

github-actions bot commented Aug 6, 2024

github-actions bot commented Aug 6, 2024

github-actions bot commented Aug 7, 2024

sfc-gh-jcieslak Aug 7, 2024 •

edited

Loading

sfc-gh-jcieslak Aug 7, 2024 •

edited

Loading