From bd98940054cd374f0a5cb5e6c092867f9c9b9d19 Mon Sep 17 00:00:00 2001 From: Sonya Huang Date: Sun, 11 Feb 2024 23:36:04 -0500 Subject: [PATCH] Apply PR feedback on initialze/refresh_mode * Add type and validations for DynamicTableInitialize options * Update type and add validations for DynamicTableRefreshMode options * Set default values for both initialize and refresh_mode * No longer use DiffSuppressFunc since the default values should match when not specified * Add separate test for initialize and refresh_mode in dynamic table integration test * Add additional steps in dynamic table acceptance test to show what happends when initialize and/or refresh_mode is changed after the dynamic table is already created. * To support the above, read the created_on date from existing dynamic tables as a way to test whether a step has replaced the dynamic table --- pkg/resources/dynamic_table.go | 56 ++++++------ .../dynamic_table_acceptance_test.go | 87 ++++++++++++++++--- .../TestAcc_DynamicTable_basic/1/test.tf | 8 +- .../TestAcc_DynamicTable_basic/1/variables.tf | 8 -- .../TestAcc_DynamicTable_basic/3/test.tf | 7 +- .../TestAcc_DynamicTable_basic/3/variables.tf | 4 + .../TestAcc_DynamicTable_basic/4/test.tf | 27 ++++++ .../TestAcc_DynamicTable_basic/4/variables.tf | 37 ++++++++ .../TestAcc_DynamicTable_basic/5/test.tf | 25 ++++++ .../TestAcc_DynamicTable_basic/5/variables.tf | 29 +++++++ pkg/sdk/dynamic_table.go | 12 +++ .../testint/dynamic_table_integration_test.go | 29 ++++++- 12 files changed, 275 insertions(+), 54 deletions(-) create mode 100644 pkg/resources/testdata/TestAcc_DynamicTable_basic/4/test.tf create mode 100644 pkg/resources/testdata/TestAcc_DynamicTable_basic/4/variables.tf create mode 100644 pkg/resources/testdata/TestAcc_DynamicTable_basic/5/test.tf create mode 100644 pkg/resources/testdata/TestAcc_DynamicTable_basic/5/variables.tf diff --git a/pkg/resources/dynamic_table.go b/pkg/resources/dynamic_table.go index 488dc9076c..a46916a359 100644 --- a/pkg/resources/dynamic_table.go +++ b/pkg/resources/dynamic_table.go @@ -4,14 +4,19 @@ import ( "context" "database/sql" "log" + "regexp" "strings" + "time" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/snowflake" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" ) +var refreshModePattern = regexp.MustCompile(`refresh_mode = '(\w+)'`) + var dynamicTableSchema = map[string]*schema.Schema{ "or_replace": { Type: schema.TypeBool, @@ -75,32 +80,25 @@ var dynamicTableSchema = map[string]*schema.Schema{ Description: "Specifies a comment for the dynamic table.", }, "refresh_mode": { - Type: schema.TypeString, - Optional: true, - Description: "INCREMENTAL if the dynamic table will use incremental refreshes, or FULL if it will recompute the whole table on every refresh.", - ForceNew: true, - DiffSuppressFunc: func(_, oldValue, newValue string, d *schema.ResourceData) bool { - switch { - case oldValue == "INCREMENTAL" && newValue == "FULL": - return false - case oldValue == "FULL" && newValue == "INCREMENTAL": - return false - case oldValue == "" && newValue != "": - return false - default: - return true - } - }, - DiffSuppressOnRefresh: true, + Type: schema.TypeString, + Optional: true, + Default: sdk.DynamicTableRefreshModeAuto, + Description: "INCREMENTAL to use incremental refreshes, FULL to recompute the whole table on every refresh, or AUTO to let Snowflake decide.", + ValidateFunc: validation.StringInSlice(sdk.AsStringList(sdk.AllDynamicRefreshModes), true), + ForceNew: true, }, "initialize": { + Type: schema.TypeString, + Optional: true, + Default: sdk.DynamicTableInitializeOnCreate, + Description: "Initialize trigger for the dynamic table. Can only be set on creation.", + ValidateFunc: validation.StringInSlice(sdk.AsStringList(sdk.AllDynamicTableInitializes), true), + ForceNew: true, + }, + "created_on": { Type: schema.TypeString, - Optional: true, - Description: "Initialize trigger for the dynamic table. Can only be set on creation.", - ForceNew: true, - DiffSuppressFunc: func(k, oldValue, newValue string, d *schema.ResourceData) bool { - return oldValue == "ON_CREATE" && newValue == "" - }, + Description: "Time when this dynamic table was created.", + Computed: true, }, "cluster_by": { Type: schema.TypeString, @@ -231,6 +229,15 @@ func ReadDynamicTable(d *schema.ResourceData, meta interface{}) error { return err } } + m := refreshModePattern.FindStringSubmatch(dynamicTable.Text) + if len(m) > 1 { + if err := d.Set("refresh_mode", m[1]); err != nil { + return err + } + } + if err := d.Set("created_on", dynamicTable.CreatedOn.Format(time.RFC3339)); err != nil { + return err + } if err := d.Set("cluster_by", dynamicTable.ClusterBy); err != nil { return err } @@ -243,9 +250,6 @@ func ReadDynamicTable(d *schema.ResourceData, meta interface{}) error { if err := d.Set("owner", dynamicTable.Owner); err != nil { return err } - if err := d.Set("refresh_mode", string(dynamicTable.RefreshMode)); err != nil { - return err - } if err := d.Set("refresh_mode_reason", dynamicTable.RefreshModeReason); err != nil { return err } diff --git a/pkg/resources/dynamic_table_acceptance_test.go b/pkg/resources/dynamic_table_acceptance_test.go index 1f248cbee2..6ebe311616 100644 --- a/pkg/resources/dynamic_table_acceptance_test.go +++ b/pkg/resources/dynamic_table_acceptance_test.go @@ -24,21 +24,29 @@ func TestAcc_DynamicTable_basic(t *testing.T) { tableName := name + "_table" m := func() map[string]config.Variable { return map[string]config.Variable{ - "name": config.StringVariable(name), - "database": config.StringVariable(acc.TestDatabaseName), - "schema": config.StringVariable(acc.TestSchemaName), - "warehouse": config.StringVariable(acc.TestWarehouseName), - "initialize": config.StringVariable("ON_SCHEDULE"), - "refresh_mode": config.StringVariable("FULL"), - "query": config.StringVariable(fmt.Sprintf(`select "id" from "%v"."%v"."%v"`, acc.TestDatabaseName, acc.TestSchemaName, tableName)), - "comment": config.StringVariable("Terraform acceptance test"), - "table_name": config.StringVariable(tableName), + "name": config.StringVariable(name), + "database": config.StringVariable(acc.TestDatabaseName), + "schema": config.StringVariable(acc.TestSchemaName), + "warehouse": config.StringVariable(acc.TestWarehouseName), + "query": config.StringVariable(fmt.Sprintf(`select "id" from "%v"."%v"."%v"`, acc.TestDatabaseName, acc.TestSchemaName, tableName)), + "comment": config.StringVariable("Terraform acceptance test"), + "table_name": config.StringVariable(tableName), } } variableSet2 := m() variableSet2["warehouse"] = config.StringVariable(acc.TestWarehouseName2) variableSet2["comment"] = config.StringVariable("Terraform acceptance test - updated") + variableSet3 := m() + variableSet3["initialize"] = config.StringVariable("ON_SCHEDULE") + + variableSet4 := m() + variableSet4["initialize"] = config.StringVariable("ON_SCHEDULE") // keep the same setting from set 3 + variableSet4["refresh_mode"] = config.StringVariable("FULL") + + // used to check whether a dynamic table was replaced + var createdOn string + resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, PreCheck: func() { acc.TestAccPreCheck(t) }, @@ -55,8 +63,8 @@ func TestAcc_DynamicTable_basic(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "database", acc.TestDatabaseName), resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName), resource.TestCheckResourceAttr(resourceName, "warehouse", acc.TestWarehouseName), - resource.TestCheckResourceAttr(resourceName, "initialize", "ON_SCHEDULE"), - resource.TestCheckResourceAttr(resourceName, "refresh_mode", "FULL"), + resource.TestCheckResourceAttr(resourceName, "initialize", "ON_CREATE"), + resource.TestCheckResourceAttr(resourceName, "refresh_mode", "AUTO"), resource.TestCheckResourceAttr(resourceName, "target_lag.#", "1"), resource.TestCheckResourceAttr(resourceName, "target_lag.0.maximum_duration", "2 minutes"), resource.TestCheckResourceAttr(resourceName, "query", fmt.Sprintf("select \"id\" from \"%v\".\"%v\".\"%v\"", acc.TestDatabaseName, acc.TestSchemaName, tableName)), @@ -76,6 +84,11 @@ func TestAcc_DynamicTable_basic(t *testing.T) { resource.TestCheckResourceAttrSet(resourceName, "is_clone"), resource.TestCheckResourceAttrSet(resourceName, "is_replica"), resource.TestCheckResourceAttrSet(resourceName, "data_timestamp"), + + resource.TestCheckResourceAttrWith(resourceName, "created_on", func(value string) error { + createdOn = value + return nil + }), ), }, // test target lag to downstream and change comment @@ -91,6 +104,58 @@ func TestAcc_DynamicTable_basic(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "target_lag.#", "1"), resource.TestCheckResourceAttr(resourceName, "target_lag.0.downstream", "true"), resource.TestCheckResourceAttr(resourceName, "comment", "Terraform acceptance test - updated"), + + resource.TestCheckResourceAttrWith(resourceName, "created_on", func(value string) error { + if value != createdOn { + return fmt.Errorf("created_on changed from %v to %v", createdOn, value) + } + return nil + }), + ), + }, + // test changing initialize setting + { + ConfigDirectory: config.TestStepDirectory(), + ConfigVariables: variableSet3, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "name", name), + resource.TestCheckResourceAttr(resourceName, "database", acc.TestDatabaseName), + resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName), + resource.TestCheckResourceAttr(resourceName, "initialize", "ON_SCHEDULE"), + resource.TestCheckResourceAttr(resourceName, "refresh_mode", "AUTO"), + resource.TestCheckResourceAttr(resourceName, "target_lag.#", "1"), + resource.TestCheckResourceAttr(resourceName, "target_lag.0.downstream", "true"), + resource.TestCheckResourceAttr(resourceName, "comment", "Terraform acceptance test"), + + resource.TestCheckResourceAttrWith(resourceName, "created_on", func(value string) error { + if value == createdOn { + return fmt.Errorf("expected created_on to change but was not changed") + } + createdOn = value + return nil + }), + ), + }, + // test changing refresh_mode setting + { + ConfigDirectory: config.TestStepDirectory(), + ConfigVariables: variableSet4, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "name", name), + resource.TestCheckResourceAttr(resourceName, "database", acc.TestDatabaseName), + resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName), + resource.TestCheckResourceAttr(resourceName, "initialize", "ON_SCHEDULE"), + resource.TestCheckResourceAttr(resourceName, "refresh_mode", "FULL"), + resource.TestCheckResourceAttr(resourceName, "target_lag.#", "1"), + resource.TestCheckResourceAttr(resourceName, "target_lag.0.downstream", "true"), + resource.TestCheckResourceAttr(resourceName, "comment", "Terraform acceptance test"), + + resource.TestCheckResourceAttrWith(resourceName, "created_on", func(value string) error { + if value == createdOn { + return fmt.Errorf("expected created_on to change but was not changed") + } + return nil + }), ), }, // test import diff --git a/pkg/resources/testdata/TestAcc_DynamicTable_basic/1/test.tf b/pkg/resources/testdata/TestAcc_DynamicTable_basic/1/test.tf index ead1a5f300..7b83d80292 100644 --- a/pkg/resources/testdata/TestAcc_DynamicTable_basic/1/test.tf +++ b/pkg/resources/testdata/TestAcc_DynamicTable_basic/1/test.tf @@ -19,9 +19,7 @@ resource "snowflake_dynamic_table" "dt" { target_lag { maximum_duration = "2 minutes" } - warehouse = var.warehouse - query = var.query - comment = var.comment - refresh_mode = var.refresh_mode - initialize = var.initialize + warehouse = var.warehouse + query = var.query + comment = var.comment } diff --git a/pkg/resources/testdata/TestAcc_DynamicTable_basic/1/variables.tf b/pkg/resources/testdata/TestAcc_DynamicTable_basic/1/variables.tf index cd55a7cfd8..81bcd62944 100644 --- a/pkg/resources/testdata/TestAcc_DynamicTable_basic/1/variables.tf +++ b/pkg/resources/testdata/TestAcc_DynamicTable_basic/1/variables.tf @@ -24,14 +24,6 @@ variable "comment" { type = string } -variable "refresh_mode" { - type = string -} - -variable "initialize" { - type = string -} - variable "table_name" { type = string } diff --git a/pkg/resources/testdata/TestAcc_DynamicTable_basic/3/test.tf b/pkg/resources/testdata/TestAcc_DynamicTable_basic/3/test.tf index 5d2ea4382d..1e30d04bd0 100644 --- a/pkg/resources/testdata/TestAcc_DynamicTable_basic/3/test.tf +++ b/pkg/resources/testdata/TestAcc_DynamicTable_basic/3/test.tf @@ -19,7 +19,8 @@ resource "snowflake_dynamic_table" "dt" { target_lag { downstream = true } - warehouse = var.warehouse - query = var.query - comment = var.comment + warehouse = var.warehouse + query = var.query + comment = var.comment + initialize = var.initialize } diff --git a/pkg/resources/testdata/TestAcc_DynamicTable_basic/3/variables.tf b/pkg/resources/testdata/TestAcc_DynamicTable_basic/3/variables.tf index 81bcd62944..7710111ac7 100644 --- a/pkg/resources/testdata/TestAcc_DynamicTable_basic/3/variables.tf +++ b/pkg/resources/testdata/TestAcc_DynamicTable_basic/3/variables.tf @@ -24,6 +24,10 @@ variable "comment" { type = string } +variable "initialize" { + type = string +} + variable "table_name" { type = string } diff --git a/pkg/resources/testdata/TestAcc_DynamicTable_basic/4/test.tf b/pkg/resources/testdata/TestAcc_DynamicTable_basic/4/test.tf new file mode 100644 index 0000000000..d20449d77c --- /dev/null +++ b/pkg/resources/testdata/TestAcc_DynamicTable_basic/4/test.tf @@ -0,0 +1,27 @@ + + +resource "snowflake_table" "t" { + database = var.database + schema = var.schema + name = var.table_name + change_tracking = true + column { + name = "id" + type = "NUMBER(38,0)" + } +} + +resource "snowflake_dynamic_table" "dt" { + depends_on = [snowflake_table.t] + name = var.name + database = var.database + schema = var.schema + target_lag { + downstream = true + } + warehouse = var.warehouse + query = var.query + comment = var.comment + refresh_mode = var.refresh_mode + initialize = var.initialize +} diff --git a/pkg/resources/testdata/TestAcc_DynamicTable_basic/4/variables.tf b/pkg/resources/testdata/TestAcc_DynamicTable_basic/4/variables.tf new file mode 100644 index 0000000000..cd55a7cfd8 --- /dev/null +++ b/pkg/resources/testdata/TestAcc_DynamicTable_basic/4/variables.tf @@ -0,0 +1,37 @@ + + +variable "name" { + type = string +} + +variable "database" { + type = string +} + +variable "schema" { + type = string +} + +variable "warehouse" { + type = string +} + +variable "query" { + type = string +} + +variable "comment" { + type = string +} + +variable "refresh_mode" { + type = string +} + +variable "initialize" { + type = string +} + +variable "table_name" { + type = string +} diff --git a/pkg/resources/testdata/TestAcc_DynamicTable_basic/5/test.tf b/pkg/resources/testdata/TestAcc_DynamicTable_basic/5/test.tf new file mode 100644 index 0000000000..5d2ea4382d --- /dev/null +++ b/pkg/resources/testdata/TestAcc_DynamicTable_basic/5/test.tf @@ -0,0 +1,25 @@ + + +resource "snowflake_table" "t" { + database = var.database + schema = var.schema + name = var.table_name + change_tracking = true + column { + name = "id" + type = "NUMBER(38,0)" + } +} + +resource "snowflake_dynamic_table" "dt" { + depends_on = [snowflake_table.t] + name = var.name + database = var.database + schema = var.schema + target_lag { + downstream = true + } + warehouse = var.warehouse + query = var.query + comment = var.comment +} diff --git a/pkg/resources/testdata/TestAcc_DynamicTable_basic/5/variables.tf b/pkg/resources/testdata/TestAcc_DynamicTable_basic/5/variables.tf new file mode 100644 index 0000000000..81bcd62944 --- /dev/null +++ b/pkg/resources/testdata/TestAcc_DynamicTable_basic/5/variables.tf @@ -0,0 +1,29 @@ + + +variable "name" { + type = string +} + +variable "database" { + type = string +} + +variable "schema" { + type = string +} + +variable "warehouse" { + type = string +} + +variable "query" { + type = string +} + +variable "comment" { + type = string +} + +variable "table_name" { + type = string +} diff --git a/pkg/sdk/dynamic_table.go b/pkg/sdk/dynamic_table.go index 75cef327f6..013778c829 100644 --- a/pkg/sdk/dynamic_table.go +++ b/pkg/sdk/dynamic_table.go @@ -71,10 +71,22 @@ type showDynamicTableOptions struct { type DynamicTableRefreshMode string const ( + DynamicTableRefreshModeAuto DynamicTableRefreshMode = "AUTO" DynamicTableRefreshModeIncremental DynamicTableRefreshMode = "INCREMENTAL" DynamicTableRefreshModeFull DynamicTableRefreshMode = "FULL" ) +var AllDynamicRefreshModes = []DynamicTableRefreshMode{DynamicTableRefreshModeAuto, DynamicTableRefreshModeIncremental, DynamicTableRefreshModeFull} + +type DynamicTableInitialize string + +const ( + DynamicTableInitializeOnCreate DynamicTableInitialize = "ON_CREATE" + DynamicTableInitializeOnSchedule DynamicTableInitialize = "ON_SCHEDULE" +) + +var AllDynamicTableInitializes = []DynamicTableInitialize{DynamicTableInitializeOnCreate, DynamicTableInitializeOnSchedule} + type DynamicTableSchedulingState string const ( diff --git a/pkg/sdk/testint/dynamic_table_integration_test.go b/pkg/sdk/testint/dynamic_table_integration_test.go index fd0adf7ab5..32c3f7ae3d 100644 --- a/pkg/sdk/testint/dynamic_table_integration_test.go +++ b/pkg/sdk/testint/dynamic_table_integration_test.go @@ -54,6 +54,32 @@ func TestInt_DynamicTableCreateAndDrop(t *testing.T) { } query := "select id from " + tableTest.ID().FullyQualifiedName() comment := random.Comment() + err := client.DynamicTables.Create(ctx, sdk.NewCreateDynamicTableRequest(name, testWarehouse(t).ID(), targetLag, query).WithOrReplace(true).WithComment(&comment)) + require.NoError(t, err) + t.Cleanup(func() { + err = client.DynamicTables.Drop(ctx, sdk.NewDropDynamicTableRequest(name)) + require.NoError(t, err) + }) + entities, err := client.DynamicTables.Show(ctx, sdk.NewShowDynamicTableRequest().WithLike(&sdk.Like{Pattern: sdk.String(name.Name())})) + require.NoError(t, err) + require.Equal(t, 1, len(entities)) + + entity := entities[0] + require.Equal(t, name.Name(), entity.Name) + require.Equal(t, testWarehouse(t).ID().Name(), entity.Warehouse) + require.Equal(t, "DOWNSTREAM", entity.TargetLag) + require.Equal(t, "INCREMENTAL", entity.RefreshMode) + require.Contains(t, entity.Text, "initialize = 'ON_SCHEDULE'") + require.Contains(t, entity.Text, "refresh_mode = 'AUTO'") + }) + + t.Run("test complete with refresh mode and initialize", func(t *testing.T) { + name := sdk.NewSchemaObjectIdentifier(testDb(t).Name, testSchema(t).Name, random.String()) + targetLag := sdk.TargetLag{ + MaximumDuration: sdk.String("2 minutes"), + } + query := "select id from " + tableTest.ID().FullyQualifiedName() + comment := random.Comment() refreshMode := "FULL" initialize := "ON_SCHEDULE" err := client.DynamicTables.Create(ctx, sdk.NewCreateDynamicTableRequest(name, testWarehouse(t).ID(), targetLag, query).WithOrReplace(true).WithInitialize(&initialize).WithRefreshMode(&refreshMode).WithComment(&comment)) @@ -71,7 +97,8 @@ func TestInt_DynamicTableCreateAndDrop(t *testing.T) { require.Equal(t, testWarehouse(t).ID().Name(), entity.Warehouse) require.Equal(t, "DOWNSTREAM", entity.TargetLag) require.Equal(t, "FULL", entity.RefreshMode) - require.Contains(t, entity.Text, "ON_SCHEDULE") + require.Contains(t, entity.Text, "initialize = 'ON_SCHEDULE'") + require.Contains(t, entity.Text, "refresh_mode = 'FULL'") }) }