From c03f171dcb2730b11456cb0e548406c403f9cfa4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Fri, 22 Nov 2024 15:02:16 +0100 Subject: [PATCH] changes after review --- MIGRATION_GUIDE.md | 22 ++- docs/resources/task.md | 12 +- .../assert/resource_assertions.go | 10 + .../resourceassert/task_resource_ext.go | 7 +- .../notification_integration_client.go | 12 ++ pkg/resources/task.go | 22 +-- pkg/resources/task_acceptance_test.go | 174 ++++++++++++------ pkg/resources/task_state_upgraders.go | 18 +- pkg/sdk/tasks_gen.go | 3 +- pkg/sdk/tasks_gen_test.go | 14 +- 10 files changed, 188 insertions(+), 106 deletions(-) diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index e78f9ecbae..b0976107d9 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -67,7 +67,8 @@ New fields: - Added support for finalizer tasks with `finalize` field. It conflicts with `after` and `schedule` (see [finalizer tasks](https://docs.snowflake.com/en/user-guide/tasks-graphs#release-and-cleanup-of-task-graphs)). Changes: -- `enabled` field changed to `started` and type changed to string with only boolean values available (see ["empty" values](./v1-preparations/CHANGES_BEFORE_V1.md#empty-values)). +- `enabled` field changed to `started` and type changed to string with only boolean values available (see ["empty" values](./v1-preparations/CHANGES_BEFORE_V1.md#empty-values)). It is also now required field, so make sure it's explicitly set (previously it was optional with the default value set to `false`). +- `allow_overlapping_execution` type was changed to string with only boolean values available (see ["empty" values](./v1-preparations/CHANGES_BEFORE_V1.md#empty-values)). Previously, it had the default set to `false` which will be migrated. If nothing will be set the provider will plan the change to `default` value. If you want to make sure it's turned off, set it explicitly to `false`. Before: ```terraform @@ -130,7 +131,24 @@ resource "snowflake_task" "example" { } ``` -- `after` field type was changed from `list` to `set`. No changes in configuration are necessary. +- `after` field type was changed from `list` to `set` and the values were changed from names to fully qualified names. + +Before: +```terraform +resource "snowflake_task" "example" { + # ... + after = ["", snowflake_task.some_task.name] + # ... +} +``` +After: +```terraform +resource "snowflake_task" "example" { + # ... + after = ["..", snowflake_task.some_task.fully_qualified_name] + # ... +} +``` ## v0.98.0 ➞ v0.99.0 diff --git a/docs/resources/task.md b/docs/resources/task.md index 642c358454..8343f80763 100644 --- a/docs/resources/task.md +++ b/docs/resources/task.md @@ -151,16 +151,16 @@ resource "snowflake_task" "test" { ### Required -- `database` (String) The database in which to create the task. Due to technical limitations (read more [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations)), avoid using the following characters: `|`, `.`, `(`, `)`, `"` -- `name` (String) Specifies the identifier for the task; must be unique for the database and schema in which the task is created. Due to technical limitations (read more [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations)), avoid using the following characters: `|`, `.`, `(`, `)`, `"` -- `schema` (String) The schema in which to create the task. Due to technical limitations (read more [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations)), avoid using the following characters: `|`, `.`, `(`, `)`, `"` +- `database` (String) The database in which to create the task. Due to technical limitations (read more [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations)), avoid using the following characters: `|`, `.`, `"` +- `name` (String) Specifies the identifier for the task; must be unique for the database and schema in which the task is created. Due to technical limitations (read more [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations)), avoid using the following characters: `|`, `.`, `"` +- `schema` (String) The schema in which to create the task. Due to technical limitations (read more [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations)), avoid using the following characters: `|`, `.`, `"` - `sql_statement` (String) Any single SQL statement, or a call to a stored procedure, executed when the task runs. - `started` (Boolean) Specifies if the task should be started or suspended. ### Optional - `abort_detached_query` (Boolean) Specifies the action that Snowflake performs for in-progress queries if connectivity is lost due to abrupt termination of a session (e.g. network outage, browser termination, service interruption). For more information, check [ABORT_DETACHED_QUERY docs](https://docs.snowflake.com/en/sql-reference/parameters#abort-detached-query). -- `after` (Set of String) Specifies one or more predecessor tasks for the current task. Use this option to [create a DAG](https://docs.snowflake.com/en/user-guide/tasks-graphs.html#label-task-dag) of tasks or add this task to an existing DAG. A DAG is a series of tasks that starts with a scheduled root task and is linked together by dependencies. Due to technical limitations (read more [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations)), avoid using the following characters: `|`, `.`, `(`, `)`, `"` +- `after` (Set of String) Specifies one or more predecessor tasks for the current task. Use this option to [create a DAG](https://docs.snowflake.com/en/user-guide/tasks-graphs.html#label-task-dag) of tasks or add this task to an existing DAG. A DAG is a series of tasks that starts with a scheduled root task and is linked together by dependencies. Due to technical limitations (read more [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations)), avoid using the following characters: `|`, `.`, `"` - `allow_overlapping_execution` (String) By default, Snowflake ensures that only one instance of a particular DAG is allowed to run at a time, setting the parameter value to TRUE permits DAG runs to overlap. Available options are: "true" or "false". When the value is not set in the configuration the provider will put "default" there which means to use the Snowflake default for this value. - `autocommit` (Boolean) Specifies whether autocommit is enabled for the session. Autocommit determines whether a DML statement, when executed without an active transaction, is automatically committed after the statement successfully completes. For more information, see [Transactions](https://docs.snowflake.com/en/sql-reference/transactions). For more information, check [AUTOCOMMIT docs](https://docs.snowflake.com/en/sql-reference/parameters#autocommit). - `binary_input_format` (String) The format of VARCHAR values passed as input to VARCHAR-to-BINARY conversion functions. For more information, see [Binary input and output](https://docs.snowflake.com/en/sql-reference/binary-input-output). For more information, check [BINARY_INPUT_FORMAT docs](https://docs.snowflake.com/en/sql-reference/parameters#binary-input-format). @@ -178,10 +178,10 @@ resource "snowflake_task" "test" { - `date_input_format` (String) Specifies the input format for the DATE data type. For more information, see [Date and time input and output formats](https://docs.snowflake.com/en/sql-reference/date-time-input-output). For more information, check [DATE_INPUT_FORMAT docs](https://docs.snowflake.com/en/sql-reference/parameters#date-input-format). - `date_output_format` (String) Specifies the display format for the DATE data type. For more information, see [Date and time input and output formats](https://docs.snowflake.com/en/sql-reference/date-time-input-output). For more information, check [DATE_OUTPUT_FORMAT docs](https://docs.snowflake.com/en/sql-reference/parameters#date-output-format). - `enable_unload_physical_type_optimization` (Boolean) Specifies whether to set the schema for unloaded Parquet files based on the logical column data types (i.e. the types in the unload SQL query or source table) or on the unloaded column values (i.e. the smallest data types and precision that support the values in the output columns of the unload SQL statement or source table). For more information, check [ENABLE_UNLOAD_PHYSICAL_TYPE_OPTIMIZATION docs](https://docs.snowflake.com/en/sql-reference/parameters#enable-unload-physical-type-optimization). -- `error_integration` (String) Specifies the name of the notification integration used for error notifications. Due to technical limitations (read more [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations)), avoid using the following characters: `|`, `.`, `(`, `)`, `"` +- `error_integration` (String) Specifies the name of the notification integration used for error notifications. Due to technical limitations (read more [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations)), avoid using the following characters: `|`, `.`, `"` - `error_on_nondeterministic_merge` (Boolean) Specifies whether to return an error when the [MERGE](https://docs.snowflake.com/en/sql-reference/sql/merge) command is used to update or delete a target row that joins multiple source rows and the system cannot determine the action to perform on the target row. For more information, check [ERROR_ON_NONDETERMINISTIC_MERGE docs](https://docs.snowflake.com/en/sql-reference/parameters#error-on-nondeterministic-merge). - `error_on_nondeterministic_update` (Boolean) Specifies whether to return an error when the [UPDATE](https://docs.snowflake.com/en/sql-reference/sql/update) command is used to update a target row that joins multiple source rows and the system cannot determine the action to perform on the target row. For more information, check [ERROR_ON_NONDETERMINISTIC_UPDATE docs](https://docs.snowflake.com/en/sql-reference/parameters#error-on-nondeterministic-update). -- `finalize` (String) Specifies the name of a root task that the finalizer task is associated with. Finalizer tasks run after all other tasks in the task graph run to completion. You can define the SQL of a finalizer task to handle notifications and the release and cleanup of resources that a task graph uses. For more information, see [Release and cleanup of task graphs](https://docs.snowflake.com/en/user-guide/tasks-graphs.html#label-finalizer-task). Due to technical limitations (read more [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations)), avoid using the following characters: `|`, `.`, `(`, `)`, `"` +- `finalize` (String) Specifies the name of a root task that the finalizer task is associated with. Finalizer tasks run after all other tasks in the task graph run to completion. You can define the SQL of a finalizer task to handle notifications and the release and cleanup of resources that a task graph uses. For more information, see [Release and cleanup of task graphs](https://docs.snowflake.com/en/user-guide/tasks-graphs.html#label-finalizer-task). Due to technical limitations (read more [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations)), avoid using the following characters: `|`, `.`, `"` - `geography_output_format` (String) Display format for [GEOGRAPHY values](https://docs.snowflake.com/en/sql-reference/data-types-geospatial.html#label-data-types-geography). For more information, check [GEOGRAPHY_OUTPUT_FORMAT docs](https://docs.snowflake.com/en/sql-reference/parameters#geography-output-format). - `geometry_output_format` (String) Display format for [GEOMETRY values](https://docs.snowflake.com/en/sql-reference/data-types-geospatial.html#label-data-types-geometry). For more information, check [GEOMETRY_OUTPUT_FORMAT docs](https://docs.snowflake.com/en/sql-reference/parameters#geometry-output-format). - `jdbc_treat_timestamp_ntz_as_utc` (Boolean) Specifies how JDBC processes TIMESTAMP_NTZ values. For more information, check [JDBC_TREAT_TIMESTAMP_NTZ_AS_UTC docs](https://docs.snowflake.com/en/sql-reference/parameters#jdbc-treat-timestamp-ntz-as-utc). diff --git a/pkg/acceptance/bettertestspoc/assert/resource_assertions.go b/pkg/acceptance/bettertestspoc/assert/resource_assertions.go index 79f4e47ac0..09c8c875cc 100644 --- a/pkg/acceptance/bettertestspoc/assert/resource_assertions.go +++ b/pkg/acceptance/bettertestspoc/assert/resource_assertions.go @@ -62,6 +62,7 @@ const ( resourceAssertionTypeValuePresent = "VALUE_PRESENT" resourceAssertionTypeValueSet = "VALUE_SET" resourceAssertionTypeValueNotSet = "VALUE_NOT_SET" + resourceAssertionTypeSetElem = "SET_ELEM" ) type ResourceAssertion struct { @@ -75,6 +76,10 @@ func (r *ResourceAssert) AddAssertion(assertion ResourceAssertion) { r.assertions = append(r.assertions, assertion) } +func SetElem(fieldName string, expected string) ResourceAssertion { + return ResourceAssertion{fieldName: fieldName, expectedValue: expected, resourceAssertionType: resourceAssertionTypeSetElem} +} + func ValuePresent(fieldName string) ResourceAssertion { return ResourceAssertion{fieldName: fieldName, resourceAssertionType: resourceAssertionTypeValuePresent} } @@ -152,6 +157,11 @@ func (r *ResourceAssert) ToTerraformTestCheckFunc(t *testing.T) resource.TestChe for i, a := range r.assertions { switch a.resourceAssertionType { + case resourceAssertionTypeSetElem: + if err := resource.TestCheckTypeSetElemAttr(r.name, a.fieldName, a.expectedValue)(s); err != nil { + errCut, _ := strings.CutPrefix(err.Error(), fmt.Sprintf("%s: ", r.name)) + result = append(result, fmt.Errorf("%s %s assertion [%d/%d]: failed with error: %s", r.name, r.prefix, i+1, len(r.assertions), errCut)) + } case resourceAssertionTypeValueSet: if err := resource.TestCheckResourceAttr(r.name, a.fieldName, a.expectedValue)(s); err != nil { errCut, _ := strings.CutPrefix(err.Error(), fmt.Sprintf("%s: ", r.name)) diff --git a/pkg/acceptance/bettertestspoc/assert/resourceassert/task_resource_ext.go b/pkg/acceptance/bettertestspoc/assert/resourceassert/task_resource_ext.go index e13c43b5d9..f948594340 100644 --- a/pkg/acceptance/bettertestspoc/assert/resourceassert/task_resource_ext.go +++ b/pkg/acceptance/bettertestspoc/assert/resourceassert/task_resource_ext.go @@ -1,7 +1,6 @@ package resourceassert import ( - "fmt" "strconv" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" @@ -9,10 +8,10 @@ import ( "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" ) -func (t *TaskResourceAssert) HasAfterIdsInOrder(ids ...sdk.SchemaObjectIdentifier) *TaskResourceAssert { +func (t *TaskResourceAssert) HasAfter(ids ...sdk.SchemaObjectIdentifier) *TaskResourceAssert { t.AddAssertion(assert.ValueSet("after.#", strconv.FormatInt(int64(len(ids)), 10))) - for i, id := range ids { - t.AddAssertion(assert.ValueSet(fmt.Sprintf("after.%d", i), id.FullyQualifiedName())) + for _, id := range ids { + t.AddAssertion(assert.SetElem("after.*", id.FullyQualifiedName())) } return t } diff --git a/pkg/acceptance/helpers/notification_integration_client.go b/pkg/acceptance/helpers/notification_integration_client.go index 7b0717e43c..e8686a234d 100644 --- a/pkg/acceptance/helpers/notification_integration_client.go +++ b/pkg/acceptance/helpers/notification_integration_client.go @@ -8,6 +8,9 @@ import ( "github.com/stretchr/testify/require" ) +// TODO [SNOW-1017580]: replace with real value +const gcpPubsubSubscriptionName = "projects/project-1234/subscriptions/sub2" + type NotificationIntegrationClient struct { context *TestClientContext ids *IdsGenerator @@ -24,6 +27,15 @@ func (c *NotificationIntegrationClient) client() sdk.NotificationIntegrations { return c.context.client.NotificationIntegrations } +func (c *NotificationIntegrationClient) CreateWithGcpPubSub(t *testing.T) (*sdk.NotificationIntegration, func()) { + t.Helper() + return c.CreateWithRequest(t, sdk.NewCreateNotificationIntegrationRequest(c.ids.RandomAccountObjectIdentifier(), true). + WithAutomatedDataLoadsParams(sdk.NewAutomatedDataLoadsParamsRequest(). + WithGoogleAutoParams(sdk.NewGoogleAutoParamsRequest(gcpPubsubSubscriptionName)), + ), + ) +} + func (c *NotificationIntegrationClient) Create(t *testing.T) (*sdk.NotificationIntegration, func()) { t.Helper() ctx := context.Background() diff --git a/pkg/resources/task.go b/pkg/resources/task.go index c25d457400..5b1ca6eb2b 100644 --- a/pkg/resources/task.go +++ b/pkg/resources/task.go @@ -67,14 +67,6 @@ var taskSchema = map[string]*schema.Schema{ Description: "The warehouse the task will use. Omit this parameter to use Snowflake-managed compute resources for runs of this task. Due to Snowflake limitations warehouse identifier can consist of only upper-cased letters. (Conflicts with user_task_managed_initial_warehouse_size)", ConflictsWith: []string{"user_task_managed_initial_warehouse_size"}, }, - "user_task_managed_initial_warehouse_size": { - Type: schema.TypeString, - Optional: true, - ValidateDiagFunc: sdkValidation(sdk.ToWarehouseSize), - DiffSuppressFunc: NormalizeAndCompare(sdk.ToWarehouseSize), - Description: "Specifies the size of the compute resources to provision for the first run of the task, before a task history is available for Snowflake to determine an ideal size. Once a task has successfully completed a few runs, Snowflake ignores this parameter setting. (Conflicts with warehouse)", - ConflictsWith: []string{"warehouse"}, - }, "schedule": { Type: schema.TypeList, Optional: true, @@ -187,7 +179,7 @@ func Task() *schema.Resource { DeleteContext: DeleteTask, Description: "Resource used to manage task objects. For more information, check [task documentation](https://docs.snowflake.com/en/user-guide/tasks-intro).", - Schema: helpers.MergeMaps(taskSchema, taskParametersSchema), + Schema: collections.MergeMaps(taskSchema, taskParametersSchema), Importer: &schema.ResourceImporter{ StateContext: ImportTask, }, @@ -594,18 +586,6 @@ func ReadTask(withExternalChangesMarking bool) schema.ReadContextFunc { attributeMappedValueReadOrDefault(d, "warehouse", task.Warehouse, func(warehouse *sdk.AccountObjectIdentifier) (string, error) { return warehouse.Name(), nil }, nil), - func() error { - managedInitialWarehouseSizeIdx := slices.IndexFunc(taskParameters, func(p *sdk.Parameter) bool { - return p != nil && p.Key == string(sdk.TaskParameterUserTaskManagedInitialWarehouseSize) - }) - if managedInitialWarehouseSizeIdx == -1 { - return fmt.Errorf("unable to find user_task_managed_initial_warehouse_size parameter") - } - if taskParameters[managedInitialWarehouseSizeIdx].Level == sdk.ParameterTypeTask { - return d.Set("user_task_managed_initial_warehouse_size", taskParameters[managedInitialWarehouseSizeIdx].Value) - } - return nil - }(), func() error { if len(task.Schedule) > 0 { taskSchedule, err := sdk.ParseTaskSchedule(task.Schedule) diff --git a/pkg/resources/task_acceptance_test.go b/pkg/resources/task_acceptance_test.go index 9592dcb58d..64eb896cde 100644 --- a/pkg/resources/task_acceptance_test.go +++ b/pkg/resources/task_acceptance_test.go @@ -28,6 +28,8 @@ import ( "github.com/hashicorp/terraform-plugin-testing/tfversion" ) +// TODO(SNOW-1822118): Create more complicated tests for task + func TestAcc_Task_Basic(t *testing.T) { _ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance) acc.TestAccPreCheck(t) @@ -62,7 +64,7 @@ func TestAcc_Task_Basic(t *testing.T) { HasErrorIntegrationString(""). HasCommentString(""). HasFinalizeString(""). - HasAfterIdsInOrder(). + HasAfter(). HasWhenString(""). HasSqlStatementString(statement), resourceshowoutputassert.TaskShowOutput(t, configModel.ResourceReference()). @@ -123,7 +125,7 @@ func TestAcc_Task_Complete(t *testing.T) { currentRole := acc.TestClient().Context.CurrentRole(t) - errorNotificationIntegration, errorNotificationIntegrationCleanup := acc.TestClient().NotificationIntegration.Create(t) + errorNotificationIntegration, errorNotificationIntegrationCleanup := acc.TestClient().NotificationIntegration.CreateWithGcpPubSub(t) t.Cleanup(errorNotificationIntegrationCleanup) id := acc.TestClient().Ids.RandomSchemaObjectIdentifier() @@ -237,7 +239,7 @@ func TestAcc_Task_Updates(t *testing.T) { warehouse, warehouseCleanup := acc.TestClient().Warehouse.CreateWarehouse(t) t.Cleanup(warehouseCleanup) - errorNotificationIntegration, errorNotificationIntegrationCleanup := acc.TestClient().NotificationIntegration.Create(t) + errorNotificationIntegration, errorNotificationIntegrationCleanup := acc.TestClient().NotificationIntegration.CreateWithGcpPubSub(t) t.Cleanup(errorNotificationIntegrationCleanup) taskConfig := `{"output_dir": "/temp/test_directory/", "learning_rate": 0.1}` @@ -276,7 +278,7 @@ func TestAcc_Task_Updates(t *testing.T) { HasErrorIntegrationString(""). HasCommentString(""). HasFinalizeString(""). - HasAfterIdsInOrder(). + HasAfter(). HasWhenString(""). HasSqlStatementString(statement), resourceshowoutputassert.TaskShowOutput(t, basicConfigModel.ResourceReference()). @@ -326,7 +328,7 @@ func TestAcc_Task_Updates(t *testing.T) { HasErrorIntegrationString(errorNotificationIntegration.ID().Name()). HasCommentString(comment). HasFinalizeString(""). - HasAfterIdsInOrder(). + HasAfter(). HasWhenString(condition). HasSqlStatementString(newStatement), resourceshowoutputassert.TaskShowOutput(t, completeConfigModel.ResourceReference()). @@ -375,7 +377,7 @@ func TestAcc_Task_Updates(t *testing.T) { HasErrorIntegrationString(""). HasCommentString(""). HasFinalizeString(""). - HasAfterIdsInOrder(). + HasAfter(). HasWhenString(""). HasSqlStatementString(statement), resourceshowoutputassert.TaskShowOutput(t, basicConfigModel.ResourceReference()). @@ -441,12 +443,11 @@ func TestAcc_Task_UpdatesInComplexDAG(t *testing.T) { )) comment := random.Comment() - basicConfigModelAfterUpdate := model.TaskWithId("test", child3Id, true, "SELECT 1"). + basicConfigModelAfterUpdate := model.TaskWithId("test", child3Id, true, "SELECT 123"). WithAfterValue(configvariable.SetVariable( configvariable.StringVariable(child1.ID().FullyQualifiedName()), configvariable.StringVariable(child2.ID().FullyQualifiedName()), )). - WithSqlStatement("SELECT 123"). // Overrides sql_statement WithComment(comment) resource.Test(t, resource.TestCase{ @@ -466,6 +467,7 @@ func TestAcc_Task_UpdatesInComplexDAG(t *testing.T) { HasSchemaString(child3Id.SchemaName()). HasNameString(child3Id.Name()). HasStartedString(r.BooleanTrue). + HasAfter(child1.ID(), child2.ID()). HasSqlStatementString("SELECT 1"), resourceshowoutputassert.TaskShowOutput(t, basicConfigModel.ResourceReference()). HasCreatedOnNotEmpty(). @@ -487,6 +489,7 @@ func TestAcc_Task_UpdatesInComplexDAG(t *testing.T) { HasNameString(child3Id.Name()). HasStartedString(r.BooleanTrue). HasCommentString(comment). + HasAfter(child1.ID(), child2.ID()). HasSqlStatementString("SELECT 123"), resourceshowoutputassert.TaskShowOutput(t, basicConfigModelAfterUpdate.ResourceReference()). HasCreatedOnNotEmpty(). @@ -577,7 +580,7 @@ func TestAcc_Task_ExternalChanges(t *testing.T) { warehouse, warehouseCleanup := acc.TestClient().Warehouse.CreateWarehouse(t) t.Cleanup(warehouseCleanup) - errorNotificationIntegration, errorNotificationIntegrationCleanup := acc.TestClient().NotificationIntegration.Create(t) + errorNotificationIntegration, errorNotificationIntegrationCleanup := acc.TestClient().NotificationIntegration.CreateWithGcpPubSub(t) t.Cleanup(errorNotificationIntegrationCleanup) taskConfig := `{"output_dir": "/temp/test_directory/", "learning_rate": 0.1}` @@ -618,7 +621,7 @@ func TestAcc_Task_ExternalChanges(t *testing.T) { HasErrorIntegrationString(errorNotificationIntegration.ID().Name()). HasCommentString(comment). HasFinalizeString(""). - HasAfterIdsInOrder(). + HasAfter(). HasWhenString(condition). HasSqlStatementString(statement), resourceshowoutputassert.TaskShowOutput(t, completeConfigModel.ResourceReference()). @@ -680,7 +683,7 @@ func TestAcc_Task_ExternalChanges(t *testing.T) { HasErrorIntegrationString(errorNotificationIntegration.ID().Name()). HasCommentString(comment). HasFinalizeString(""). - HasAfterIdsInOrder(). + HasAfter(). HasWhenString(condition). HasSqlStatementString(statement), resourceshowoutputassert.TaskShowOutput(t, completeConfigModel.ResourceReference()). @@ -724,7 +727,7 @@ func TestAcc_Task_ExternalChanges(t *testing.T) { HasErrorIntegrationString(""). HasCommentString(""). HasFinalizeString(""). - HasAfterIdsInOrder(). + HasAfter(). HasWhenString(""). HasSqlStatementString(statement), resourceshowoutputassert.TaskShowOutput(t, basicConfigModel.ResourceReference()). @@ -786,7 +789,7 @@ func TestAcc_Task_ExternalChanges(t *testing.T) { HasErrorIntegrationString(""). HasCommentString(""). HasFinalizeString(""). - HasAfterIdsInOrder(). + HasAfter(). HasWhenString(""). HasSqlStatementString(statement), resourceshowoutputassert.TaskShowOutput(t, basicConfigModel.ResourceReference()). @@ -1622,7 +1625,7 @@ func TestAcc_Task_ConvertStandaloneTaskToSubtask(t *testing.T) { HasScheduleMinutes(5). HasState(sdk.TaskStateStarted), resourceassert.TaskResource(t, childTaskModel.ResourceReference()). - HasAfterIdsInOrder(id). + HasAfter(id). HasStartedString(r.BooleanTrue), resourceshowoutputassert.TaskShowOutput(t, childTaskModel.ResourceReference()). HasPredecessors(id). @@ -1800,7 +1803,7 @@ func TestAcc_Task_SwitchScheduledWithAfter(t *testing.T) { resourceassert.TaskResource(t, childTaskConfigModel.ResourceReference()). HasStartedString(r.BooleanTrue). HasScheduleMinutes(schedule). - HasAfterIdsInOrder(). + HasAfter(). HasSuspendTaskAfterNumFailuresString("10"), ), }, @@ -1815,7 +1818,7 @@ func TestAcc_Task_SwitchScheduledWithAfter(t *testing.T) { resourceassert.TaskResource(t, childTaskConfigModelWithAfter.ResourceReference()). HasStartedString(r.BooleanTrue). HasNoScheduleSet(). - HasAfterIdsInOrder(rootId). + HasAfter(rootId). HasSuspendTaskAfterNumFailuresString("10"), ), }, @@ -1830,7 +1833,7 @@ func TestAcc_Task_SwitchScheduledWithAfter(t *testing.T) { resourceassert.TaskResource(t, childTaskConfigModel.ResourceReference()). HasStartedString(r.BooleanTrue). HasScheduleMinutes(schedule). - HasAfterIdsInOrder(). + HasAfter(). HasSuspendTaskAfterNumFailuresString("10"), ), }, @@ -1845,7 +1848,7 @@ func TestAcc_Task_SwitchScheduledWithAfter(t *testing.T) { resourceassert.TaskResource(t, childTaskConfigModelDisabled.ResourceReference()). HasStartedString(r.BooleanFalse). HasScheduleMinutes(schedule). - HasAfterIdsInOrder(). + HasAfter(). HasSuspendTaskAfterNumFailuresString("10"), ), }, @@ -1894,7 +1897,7 @@ func TestAcc_Task_WithAfter(t *testing.T) { HasScheduleMinutes(schedule), resourceassert.TaskResource(t, childTaskConfigModelWithAfter.ResourceReference()). HasStartedString(r.BooleanTrue). - HasAfterIdsInOrder(rootId), + HasAfter(rootId), ), }, { @@ -1906,7 +1909,7 @@ func TestAcc_Task_WithAfter(t *testing.T) { HasScheduleMinutes(schedule), resourceassert.TaskResource(t, childTaskConfigModelWithoutAfter.ResourceReference()). HasStartedString(r.BooleanTrue). - HasAfterIdsInOrder(), + HasAfter(), ), }, }, @@ -2142,7 +2145,7 @@ func TestAcc_Task_UpdateAfterExternally(t *testing.T) { Check: assert.AssertThat(t, resourceassert.TaskResource(t, childTaskConfigModelWithoutAfter.ResourceReference()). HasStartedString(r.BooleanTrue). - HasAfterIdsInOrder(), + HasAfter(), resourceshowoutputassert.TaskShowOutput(t, childTaskConfigModelWithoutAfter.ResourceReference()). HasState(sdk.TaskStateStarted). HasTaskRelations(sdk.TaskRelations{}), @@ -2155,7 +2158,7 @@ func TestAcc_Task_UpdateAfterExternally(t *testing.T) { Check: assert.AssertThat(t, resourceassert.TaskResource(t, childTaskConfigModelWithAfter.ResourceReference()). HasStartedString(r.BooleanTrue). - HasAfterIdsInOrder(rootId), + HasAfter(rootId), resourceshowoutputassert.TaskShowOutput(t, childTaskConfigModelWithAfter.ResourceReference()). HasState(sdk.TaskStateStarted). HasTaskRelations(sdk.TaskRelations{Predecessors: []sdk.SchemaObjectIdentifier{rootId}}), @@ -2178,7 +2181,7 @@ func TestAcc_Task_UpdateAfterExternally(t *testing.T) { Check: assert.AssertThat(t, resourceassert.TaskResource(t, childTaskConfigModelWithAfter.ResourceReference()). HasStartedString(r.BooleanTrue). - HasAfterIdsInOrder(rootId), + HasAfter(rootId), resourceshowoutputassert.TaskShowOutput(t, childTaskConfigModelWithAfter.ResourceReference()). HasState(sdk.TaskStateStarted). HasTaskRelations(sdk.TaskRelations{Predecessors: []sdk.SchemaObjectIdentifier{rootId}}), @@ -2191,7 +2194,7 @@ func TestAcc_Task_UpdateAfterExternally(t *testing.T) { Check: assert.AssertThat(t, resourceassert.TaskResource(t, childTaskConfigModelWithoutAfter.ResourceReference()). HasStartedString(r.BooleanTrue). - HasAfterIdsInOrder(), + HasAfter(), resourceshowoutputassert.TaskShowOutput(t, childTaskConfigModelWithoutAfter.ResourceReference()). HasState(sdk.TaskStateStarted). HasTaskRelations(sdk.TaskRelations{}), @@ -2244,7 +2247,7 @@ func TestAcc_Task_issue2207(t *testing.T) { HasScheduleMinutes(schedule), resourceassert.TaskResource(t, childTaskConfigModel.ResourceReference()). HasStartedString(r.BooleanTrue). - HasAfterIdsInOrder(rootId). + HasAfter(rootId). HasCommentString("abc"), ), }, @@ -2263,7 +2266,7 @@ func TestAcc_Task_issue2207(t *testing.T) { HasScheduleMinutes(schedule), resourceassert.TaskResource(t, childTaskConfigModelWithDifferentComment.ResourceReference()). HasStartedString(r.BooleanTrue). - HasAfterIdsInOrder(rootId). + HasAfter(rootId). HasCommentString("def"), ), }, @@ -2335,7 +2338,7 @@ func TestAcc_Task_issue3113(t *testing.T) { _ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance) acc.TestAccPreCheck(t) - errorNotificationIntegration, errorNotificationIntegrationCleanup := acc.TestClient().NotificationIntegration.Create(t) + errorNotificationIntegration, errorNotificationIntegrationCleanup := acc.TestClient().NotificationIntegration.CreateWithGcpPubSub(t) t.Cleanup(errorNotificationIntegrationCleanup) id := acc.TestClient().Ids.RandomSchemaObjectIdentifier() @@ -2379,18 +2382,50 @@ func TestAcc_Task_issue3113(t *testing.T) { }) } -func taskConfigWithErrorIntegration(id sdk.SchemaObjectIdentifier, errorIntegrationId sdk.AccountObjectIdentifier) string { - return fmt.Sprintf(` -resource "snowflake_task" "test" { - database = "%[1]s" - schema = "%[2]s" - name = "%[3]s" - schedule = "5 MINUTES" - sql_statement = "SELECT 1" - enabled = true - error_integration = "%[4]s" -} -`, id.DatabaseName(), id.SchemaName(), id.Name(), errorIntegrationId.Name()) +func TestAcc_Task_StateUpgrade_NoOptionalFields(t *testing.T) { + _ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance) + acc.TestAccPreCheck(t) + + id := acc.TestClient().Ids.RandomSchemaObjectIdentifier() + statement := "SELECT 1" + configModel := model.TaskWithId("test", id, false, statement) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: acc.CheckDestroy(t, resources.Task), + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "snowflake": { + VersionConstraint: "=0.98.0", + Source: "Snowflake-Labs/snowflake", + }, + }, + Config: taskNoOptionalFieldsConfigV0980(id), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_task.test", "enabled", "false"), + resource.TestCheckResourceAttr("snowflake_task.test", "allow_overlapping_execution", "false"), + ), + }, + { + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_Task/basic"), + ConfigVariables: config.ConfigVariablesFromModel(t, configModel), + Check: assert.AssertThat(t, + resourceassert.TaskResource(t, configModel.ResourceReference()). + HasFullyQualifiedNameString(id.FullyQualifiedName()). + HasDatabaseString(id.DatabaseName()). + HasSchemaString(id.SchemaName()). + HasNameString(id.Name()). + HasStartedString(r.BooleanFalse). + HasAllowOverlappingExecutionString(r.BooleanDefault), + ), + }, + }, + }) } func TestAcc_Task_StateUpgrade(t *testing.T) { @@ -2453,23 +2488,6 @@ func TestAcc_Task_StateUpgrade(t *testing.T) { }) } -func taskBasicConfigV0980(id sdk.SchemaObjectIdentifier, condition string) string { - return fmt.Sprintf(` -resource "snowflake_task" "test" { - database = "%[1]s" - schema = "%[2]s" - name = "%[3]s" - enabled = false - sql_statement = "SELECT 1" - schedule = "5 MINUTES" - allow_overlapping_execution = true - suspend_task_after_num_failures = 10 - when = "%[4]s" - user_task_managed_initial_warehouse_size = "XSMALL" -} -`, id.DatabaseName(), id.SchemaName(), id.Name(), condition) -} - func TestAcc_Task_StateUpgradeWithAfter(t *testing.T) { _ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance) acc.TestAccPreCheck(t) @@ -2526,7 +2544,7 @@ func TestAcc_Task_StateUpgradeWithAfter(t *testing.T) { HasNameString(id.Name()). HasStartedString(r.BooleanFalse). HasSqlStatementString(statement). - HasAfterIdsInOrder(rootTask.ID()). + HasAfter(rootTask.ID()). HasWarehouseString(acc.TestClient().Ids.WarehouseId().Name()). HasUserTaskTimeoutMsString("50"). HasLogLevelString(string(sdk.LogLevelInfo)). @@ -2539,6 +2557,48 @@ func TestAcc_Task_StateUpgradeWithAfter(t *testing.T) { }) } +func taskNoOptionalFieldsConfigV0980(id sdk.SchemaObjectIdentifier) string { + return fmt.Sprintf(` +resource "snowflake_task" "test" { + database = "%[1]s" + schema = "%[2]s" + name = "%[3]s" + sql_statement = "SELECT 1" +} +`, id.DatabaseName(), id.SchemaName(), id.Name()) +} + +func taskConfigWithErrorIntegration(id sdk.SchemaObjectIdentifier, errorIntegrationId sdk.AccountObjectIdentifier) string { + return fmt.Sprintf(` +resource "snowflake_task" "test" { + database = "%[1]s" + schema = "%[2]s" + name = "%[3]s" + schedule = "5 MINUTES" + sql_statement = "SELECT 1" + enabled = true + error_integration = "%[4]s" +} +`, id.DatabaseName(), id.SchemaName(), id.Name(), errorIntegrationId.Name()) +} + +func taskBasicConfigV0980(id sdk.SchemaObjectIdentifier, condition string) string { + return fmt.Sprintf(` +resource "snowflake_task" "test" { + database = "%[1]s" + schema = "%[2]s" + name = "%[3]s" + enabled = false + sql_statement = "SELECT 1" + schedule = "5 MINUTES" + allow_overlapping_execution = true + suspend_task_after_num_failures = 10 + when = "%[4]s" + user_task_managed_initial_warehouse_size = "XSMALL" +} +`, id.DatabaseName(), id.SchemaName(), id.Name(), condition) +} + func taskCompleteConfigV0980( id sdk.SchemaObjectIdentifier, rootTaskId sdk.SchemaObjectIdentifier, diff --git a/pkg/resources/task_state_upgraders.go b/pkg/resources/task_state_upgraders.go index e02512dd30..1a77858405 100644 --- a/pkg/resources/task_state_upgraders.go +++ b/pkg/resources/task_state_upgraders.go @@ -12,18 +12,22 @@ func v098TaskStateUpgrader(ctx context.Context, rawState map[string]any, meta an } rawState["condition"] = rawState["when"] - rawState["started"] = booleanStringFromBool(rawState["enabled"].(bool)) + rawState["started"] = rawState["enabled"].(bool) rawState["allow_overlapping_execution"] = booleanStringFromBool(rawState["allow_overlapping_execution"].(bool)) if rawState["after"] != nil { - newAfter := make([]string, len(rawState["after"].([]any))) - for i, name := range rawState["after"].([]any) { - newAfter[i] = sdk.NewSchemaObjectIdentifier(rawState["database"].(string), rawState["schema"].(string), name.(string)).FullyQualifiedName() + if afterSlice, okType := rawState["after"].([]any); okType { + newAfter := make([]string, len(afterSlice)) + for i, name := range afterSlice { + newAfter[i] = sdk.NewSchemaObjectIdentifier(rawState["database"].(string), rawState["schema"].(string), name.(string)).FullyQualifiedName() + } + rawState["after"] = newAfter } - rawState["after"] = newAfter } if rawState["session_parameters"] != nil { - for k, v := range rawState["session_parameters"].(map[string]any) { - rawState[k] = v + if sessionParamsMap, okType := rawState["session_parameters"].(map[string]any); okType { + for k, v := range sessionParamsMap { + rawState[k] = v + } } } delete(rawState, "session_parameters") diff --git a/pkg/sdk/tasks_gen.go b/pkg/sdk/tasks_gen.go index 203e76459b..3d6bf112bb 100644 --- a/pkg/sdk/tasks_gen.go +++ b/pkg/sdk/tasks_gen.go @@ -239,8 +239,9 @@ func ParseTaskSchedule(schedule string) (*TaskSchedule, error) { } return &TaskSchedule{Minutes: minutes}, nil + default: + return nil, fmt.Errorf("invalid schedule format: %s", schedule) } - return nil, fmt.Errorf("invalid schedule format: %s", schedule) } // DescribeTaskOptions is based on https://docs.snowflake.com/en/sql-reference/sql/desc-task. diff --git a/pkg/sdk/tasks_gen_test.go b/pkg/sdk/tasks_gen_test.go index 0d0c72181f..9422d73824 100644 --- a/pkg/sdk/tasks_gen_test.go +++ b/pkg/sdk/tasks_gen_test.go @@ -564,27 +564,26 @@ func TestParseTaskSchedule(t *testing.T) { "valid schedule: m minutes": { Schedule: "5 m", ExpectedTaskSchedule: &TaskSchedule{Minutes: 5}, - Error: "", }, "valid schedule: M minutes": { - Schedule: "5 m", + Schedule: "5 M", ExpectedTaskSchedule: &TaskSchedule{Minutes: 5}, - Error: "", }, "valid schedule: MINUTE minutes": { Schedule: "5 MINUTE", ExpectedTaskSchedule: &TaskSchedule{Minutes: 5}, - Error: "", }, "valid schedule: MINUTES minutes": { Schedule: "5 MINUTES", ExpectedTaskSchedule: &TaskSchedule{Minutes: 5}, - Error: "", }, "valid schedule: cron": { Schedule: "USING CRON * * * * * UTC", ExpectedTaskSchedule: &TaskSchedule{Cron: "* * * * * UTC"}, - Error: "", + }, + "valid schedule: cron with case sensitive location": { + Schedule: "USING CRON * * * * * America/Loc_Angeles", + ExpectedTaskSchedule: &TaskSchedule{Cron: "* * * * * America/Loc_Angeles"}, }, "invalid schedule: wrong schedule format": { Schedule: "SOME SCHEDULE", @@ -596,11 +595,10 @@ func TestParseTaskSchedule(t *testing.T) { ExpectedTaskSchedule: nil, Error: `strconv.Atoi: parsing "A5": invalid syntax`, }, - // currently cron expressions are not validated (they are on Snowflake level) + // currently, cron expressions are not validated (they are on Snowflake level) "invalid schedule: wrong cron format": { Schedule: "USING CRON some_cron", ExpectedTaskSchedule: &TaskSchedule{Cron: "some_cron"}, - Error: "", }, }