Skip to content

Commit

Permalink
chore: Adress small task-related todos (#3243)
Browse files Browse the repository at this point in the history 
### Changes
- Addressing 4 task-related issues
- Two of them await to be reported, created a ticket for that and will
put the correct tickets when they'll be created.
- The `getPredecessors` function is still used, but the field it
populates is not used in the resource; `TaskRelations` are used instead.
  - Task parameters were already handled properly in the resource.
  • Loading branch information
@sfc-gh-jcieslak
sfc-gh-jcieslak authored Dec 4, 2024
1 parent 425787c commit 40de9ae
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 22 deletions.
2 changes: 1 addition & 1 deletion pkg/resources/task.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ func ReadTask(withExternalChangesMarking bool) schema.ReadContextFunc {
d.Set("config", task.Config),
d.Set("comment", task.Comment),
d.Set("sql_statement", task.Definition),
d.Set("after", collections.Map(task.Predecessors, sdk.SchemaObjectIdentifier.FullyQualifiedName)),
d.Set("after", collections.Map(task.TaskRelations.Predecessors, sdk.SchemaObjectIdentifier.FullyQualifiedName)),
handleTaskParameterRead(d, taskParameters),
d.Set(FullyQualifiedNameAttributeName, id.FullyQualifiedName()),
d.Set(ShowOutputAttributeName, []map[string]any{schemas.TaskToSchema(task)}),
Expand Down
2 changes: 1 addition & 1 deletion pkg/resources/task_acceptance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1724,7 +1724,7 @@ func TestAcc_Task_ConvertStandaloneTaskToFinalizer(t *testing.T) {
HasSuspendTaskAfterNumFailuresString("2"),
resourceshowoutputassert.TaskShowOutput(t, rootTaskModel.ResourceReference()).
HasScheduleMinutes(schedule).
// TODO(SNOW-1348116 - next pr): Create ticket and report; this field in task relations seems to have mixed chances of appearing (needs deeper digging, doesn't affect the resource; could be removed for now)
// TODO(SNOW-1843489): Create ticket and report; this field in task relations seems to have mixed chances of appearing (needs deeper digging, doesn't affect the resource; could be removed for now)
// HasTaskRelations(sdk.TaskRelations{FinalizerTask: &finalizerTaskId}).
HasState(sdk.TaskStateStarted),
resourceassert.TaskResource(t, childTaskModel.ResourceReference()).
Expand Down
1 change: 0 additions & 1 deletion pkg/sdk/parameters.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,6 @@ var AllUserParameters = []UserParameter{

type TaskParameter string

// TODO(SNOW-1348116 - next prs): Handle task parameters
const (
// Task Parameters
TaskParameterSuspendTaskAfterNumFailures TaskParameter = "SUSPEND_TASK_AFTER_NUM_FAILURES"
Expand Down
1 change: 0 additions & 1 deletion pkg/sdk/tasks_impl_gen.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,6 @@ func (r taskDBRow) convert() *Task {
return &task
}

// TODO(SNOW-1348116 - next prs): Remove and use Task.TaskRelations instead
func getPredecessors(predecessors string) ([]string, error) {
// Since 2022_03, Snowflake returns this as a JSON array (even empty)
// The list is formatted, e.g.:
Expand Down
36 changes: 18 additions & 18 deletions pkg/sdk/testint/tasks_gen_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -356,25 +356,25 @@ func TestInt_Tasks(t *testing.T) {
rootId := testClientHelper().Ids.RandomSchemaObjectIdentifier()
root, rootCleanup := testClientHelper().Task.CreateWithRequest(t, sdk.NewCreateTaskRequest(rootId, sql).WithSchedule("10 MINUTE"))
t.Cleanup(rootCleanup)
require.Empty(t, root.Predecessors)
require.Empty(t, root.TaskRelations.Predecessors)

t1, t1Cleanup := testClientHelper().Task.CreateWithAfter(t, rootId)
t.Cleanup(t1Cleanup)
require.Equal(t, []sdk.SchemaObjectIdentifier{rootId}, t1.Predecessors)
require.Equal(t, []sdk.SchemaObjectIdentifier{rootId}, t1.TaskRelations.Predecessors)

t2, t2Cleanup := testClientHelper().Task.CreateWithAfter(t, t1.ID(), rootId)
t.Cleanup(t2Cleanup)

require.Contains(t, t2.Predecessors, rootId)
require.Contains(t, t2.Predecessors, t1.ID())
require.Len(t, t2.Predecessors, 2)
require.Contains(t, t2.TaskRelations.Predecessors, rootId)
require.Contains(t, t2.TaskRelations.Predecessors, t1.ID())
require.Len(t, t2.TaskRelations.Predecessors, 2)

t3, t3Cleanup := testClientHelper().Task.CreateWithAfter(t, t2.ID(), t1.ID())
t.Cleanup(t3Cleanup)

require.Contains(t, t3.Predecessors, t2.ID())
require.Contains(t, t3.Predecessors, t1.ID())
require.Len(t, t3.Predecessors, 2)
require.Contains(t, t3.TaskRelations.Predecessors, t2.ID())
require.Contains(t, t3.TaskRelations.Predecessors, t1.ID())
require.Len(t, t3.TaskRelations.Predecessors, 2)

rootTasks, err := sdk.GetRootTasks(client.Tasks, ctx, rootId)
require.NoError(t, err)
Expand Down Expand Up @@ -433,19 +433,19 @@ func TestInt_Tasks(t *testing.T) {
root1Id := testClientHelper().Ids.RandomSchemaObjectIdentifier()
root1, root1Cleanup := testClientHelper().Task.CreateWithRequest(t, sdk.NewCreateTaskRequest(root1Id, sql).WithSchedule("10 MINUTE"))
t.Cleanup(root1Cleanup)
require.Empty(t, root1.Predecessors)
require.Empty(t, root1.TaskRelations.Predecessors)

root2Id := testClientHelper().Ids.RandomSchemaObjectIdentifier()
root2, root2Cleanup := testClientHelper().Task.CreateWithRequest(t, sdk.NewCreateTaskRequest(root2Id, sql).WithSchedule("10 MINUTE"))
t.Cleanup(root2Cleanup)
require.Empty(t, root2.Predecessors)
require.Empty(t, root2.TaskRelations.Predecessors)

t1, t1Cleanup := testClientHelper().Task.CreateWithAfter(t, root1.ID(), root2.ID())
t.Cleanup(t1Cleanup)

require.Contains(t, t1.Predecessors, root1Id)
require.Contains(t, t1.Predecessors, root2Id)
require.Len(t, t1.Predecessors, 2)
require.Contains(t, t1.TaskRelations.Predecessors, root1Id)
require.Contains(t, t1.TaskRelations.Predecessors, root2Id)
require.Len(t, t1.TaskRelations.Predecessors, 2)

rootTasks, err := sdk.GetRootTasks(client.Tasks, ctx, t1.ID())
require.NoError(t, err)
Expand Down Expand Up @@ -522,7 +522,7 @@ func TestInt_Tasks(t *testing.T) {
assert.Equal(t, sourceTask.Config, task.Config)
assert.Equal(t, sourceTask.Condition, task.Condition)
assert.Equal(t, sourceTask.Warehouse, task.Warehouse)
assert.Equal(t, sourceTask.Predecessors, task.Predecessors)
assert.Equal(t, sourceTask.TaskRelations.Predecessors, task.TaskRelations.Predecessors)
assert.Equal(t, sourceTask.AllowOverlappingExecution, task.AllowOverlappingExecution)
assert.Equal(t, sourceTask.Comment, task.Comment)
assert.Equal(t, sourceTask.ErrorIntegration, task.ErrorIntegration)
Expand Down Expand Up @@ -613,7 +613,7 @@ func TestInt_Tasks(t *testing.T) {
t.Cleanup(taskCleanup)

err := client.Tasks.Alter(ctx, sdk.NewAlterTaskRequest(task.ID()).WithSet(*sdk.NewTaskSetRequest().
// TODO(SNOW-1348116): Cannot set warehouse due to Snowflake error
// TODO(SNOW-1843489): Cannot set warehouse due to Snowflake error
// WithWarehouse(testClientHelper().Ids.WarehouseId()).
WithErrorIntegration(errorIntegration.ID()).
WithSessionParameters(sessionParametersSet).
Expand Down Expand Up @@ -754,23 +754,23 @@ func TestInt_Tasks(t *testing.T) {
task, taskCleanup := testClientHelper().Task.CreateWithAfter(t, rootTask.ID())
t.Cleanup(taskCleanup)

assert.Contains(t, task.Predecessors, rootTask.ID())
assert.Contains(t, task.TaskRelations.Predecessors, rootTask.ID())

err := client.Tasks.Alter(ctx, sdk.NewAlterTaskRequest(task.ID()).WithRemoveAfter([]sdk.SchemaObjectIdentifier{rootTask.ID()}))
require.NoError(t, err)

task, err = client.Tasks.ShowByID(ctx, task.ID())

require.NoError(t, err)
assert.Empty(t, task.Predecessors)
assert.Empty(t, task.TaskRelations.Predecessors)

err = client.Tasks.Alter(ctx, sdk.NewAlterTaskRequest(task.ID()).WithAddAfter([]sdk.SchemaObjectIdentifier{rootTask.ID()}))
require.NoError(t, err)

task, err = client.Tasks.ShowByID(ctx, task.ID())

require.NoError(t, err)
assert.Contains(t, task.Predecessors, rootTask.ID())
assert.Contains(t, task.TaskRelations.Predecessors, rootTask.ID())
})

t.Run("alter task: set and unset final task", func(t *testing.T) {
Expand Down

0 comments on commit 40de9ae

Please sign in to comment.