From 738d19efe7c0c64922182166117132afb0e55e28 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Fri, 19 Apr 2024 07:32:45 -0700 Subject: [PATCH 1/7] Enable job queueing by default --- bundle/config/mutator/default_queueing.go | 35 ++++++ .../config/mutator/default_queueing_test.go | 103 ++++++++++++++++++ bundle/phases/initialize.go | 1 + 3 files changed, 139 insertions(+) create mode 100644 bundle/config/mutator/default_queueing.go create mode 100644 bundle/config/mutator/default_queueing_test.go diff --git a/bundle/config/mutator/default_queueing.go b/bundle/config/mutator/default_queueing.go new file mode 100644 index 0000000000..9dee98e24d --- /dev/null +++ b/bundle/config/mutator/default_queueing.go @@ -0,0 +1,35 @@ +package mutator + +import ( + "context" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/databricks-sdk-go/service/jobs" +) + +type defaultQueueing struct{} + +func DefaultQueueing() bundle.Mutator { + return &defaultQueueing{} +} + +func (m *defaultQueueing) Name() string { + return "DefaultQueueing" +} + +// Enable queueing for jobs by default, following the behavior from API 2.2+. +// As of 2024-04, we're still using API 2.1 which has queueing disabled by default. +// This mutator makes sure queueing is enabled by default before we can adopt API 2.2. +func (m *defaultQueueing) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + r := b.Config.Resources + for i := range r.Jobs { + if r.Jobs[i].Queue != nil { + continue + } + r.Jobs[i].Queue = &jobs.QueueSettings{ + Enabled: true, + } + } + return nil +} diff --git a/bundle/config/mutator/default_queueing_test.go b/bundle/config/mutator/default_queueing_test.go new file mode 100644 index 0000000000..9bebbaa0b5 --- /dev/null +++ b/bundle/config/mutator/default_queueing_test.go @@ -0,0 +1,103 @@ +package mutator + +import ( + "context" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/databricks-sdk-go/service/jobs" + "github.com/stretchr/testify/assert" +) + +func TestDefaultQueueing(t *testing.T) { + m := DefaultQueueing() + assert.IsType(t, &defaultQueueing{}, m) +} + +func TestName(t *testing.T) { + m := DefaultQueueing() + assert.Equal(t, "DefaultQueueing", m.Name()) +} + +func TestApplyNoJobs(t *testing.T) { + m := DefaultQueueing().(*defaultQueueing) + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{}, + }, + } + d := m.Apply(context.Background(), b) + assert.Len(t, d, 0) + assert.Len(t, b.Config.Resources.Jobs, 0) +} + +func TestApplyJobsAlreadyEnabled(t *testing.T) { + m := DefaultQueueing().(*defaultQueueing) + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job": { + JobSettings: &jobs.JobSettings{ + Queue: &jobs.QueueSettings{Enabled: true}, + }, + }, + }, + }, + }, + } + d := m.Apply(context.Background(), b) + assert.Len(t, d, 0) + assert.True(t, b.Config.Resources.Jobs["job"].Queue.Enabled) +} + +func TestApplyEnableQueueing(t *testing.T) { + m := DefaultQueueing().(*defaultQueueing) + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job": { + JobSettings: &jobs.JobSettings{}, + }, + }, + }, + }, + } + d := m.Apply(context.Background(), b) + assert.Len(t, d, 0) + assert.NotNil(t, b.Config.Resources.Jobs["job"].Queue) + assert.True(t, b.Config.Resources.Jobs["job"].Queue.Enabled) +} + +func TestApplyWithMultipleJobs(t *testing.T) { + m := DefaultQueueing().(*defaultQueueing) + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job1": { + JobSettings: &jobs.JobSettings{ + Queue: &jobs.QueueSettings{Enabled: false}, + }, + }, + "job2": { + JobSettings: &jobs.JobSettings{}, + }, + "job3": { + JobSettings: &jobs.JobSettings{ + Queue: &jobs.QueueSettings{Enabled: true}, + }, + }, + }, + }, + }, + } + d := m.Apply(context.Background(), b) + assert.Len(t, d, 0) + assert.False(t, b.Config.Resources.Jobs["job1"].Queue.Enabled) + assert.True(t, b.Config.Resources.Jobs["job2"].Queue.Enabled) + assert.True(t, b.Config.Resources.Jobs["job3"].Queue.Enabled) +} diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index d6a1b95da9..2f5eab302c 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -38,6 +38,7 @@ func Initialize() bundle.Mutator { mutator.SetRunAs(), mutator.OverrideCompute(), mutator.ProcessTargetMode(), + mutator.DefaultQueueing(), mutator.ExpandPipelineGlobPaths(), mutator.TranslatePaths(), python.WrapperWarning(), From 267d7fb38fd9f78fe6964f9fb45fa25a71f55248 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Mon, 22 Apr 2024 11:47:17 +0200 Subject: [PATCH 2/7] Cleanup --- bundle/config/mutator/default_queueing.go | 3 +++ .../config/mutator/default_queueing_test.go | 20 ++++++------------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/bundle/config/mutator/default_queueing.go b/bundle/config/mutator/default_queueing.go index 9dee98e24d..ead77c7a86 100644 --- a/bundle/config/mutator/default_queueing.go +++ b/bundle/config/mutator/default_queueing.go @@ -24,6 +24,9 @@ func (m *defaultQueueing) Name() string { func (m *defaultQueueing) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { r := b.Config.Resources for i := range r.Jobs { + if r.Jobs[i].JobSettings == nil { + r.Jobs[i].JobSettings = &jobs.JobSettings{} + } if r.Jobs[i].Queue != nil { continue } diff --git a/bundle/config/mutator/default_queueing_test.go b/bundle/config/mutator/default_queueing_test.go index 9bebbaa0b5..26b98328b4 100644 --- a/bundle/config/mutator/default_queueing_test.go +++ b/bundle/config/mutator/default_queueing_test.go @@ -22,19 +22,17 @@ func TestName(t *testing.T) { } func TestApplyNoJobs(t *testing.T) { - m := DefaultQueueing().(*defaultQueueing) b := &bundle.Bundle{ Config: config.Root{ Resources: config.Resources{}, }, } - d := m.Apply(context.Background(), b) + d := bundle.Apply(context.Background(), b, DefaultQueueing()) assert.Len(t, d, 0) assert.Len(t, b.Config.Resources.Jobs, 0) } func TestApplyJobsAlreadyEnabled(t *testing.T) { - m := DefaultQueueing().(*defaultQueueing) b := &bundle.Bundle{ Config: config.Root{ Resources: config.Resources{ @@ -48,32 +46,28 @@ func TestApplyJobsAlreadyEnabled(t *testing.T) { }, }, } - d := m.Apply(context.Background(), b) + d := bundle.Apply(context.Background(), b, DefaultQueueing()) assert.Len(t, d, 0) assert.True(t, b.Config.Resources.Jobs["job"].Queue.Enabled) } func TestApplyEnableQueueing(t *testing.T) { - m := DefaultQueueing().(*defaultQueueing) b := &bundle.Bundle{ Config: config.Root{ Resources: config.Resources{ Jobs: map[string]*resources.Job{ - "job": { - JobSettings: &jobs.JobSettings{}, - }, + "job": {}, }, }, }, } - d := m.Apply(context.Background(), b) + d := bundle.Apply(context.Background(), b, DefaultQueueing()) assert.Len(t, d, 0) assert.NotNil(t, b.Config.Resources.Jobs["job"].Queue) assert.True(t, b.Config.Resources.Jobs["job"].Queue.Enabled) } func TestApplyWithMultipleJobs(t *testing.T) { - m := DefaultQueueing().(*defaultQueueing) b := &bundle.Bundle{ Config: config.Root{ Resources: config.Resources{ @@ -83,9 +77,7 @@ func TestApplyWithMultipleJobs(t *testing.T) { Queue: &jobs.QueueSettings{Enabled: false}, }, }, - "job2": { - JobSettings: &jobs.JobSettings{}, - }, + "job2": {}, "job3": { JobSettings: &jobs.JobSettings{ Queue: &jobs.QueueSettings{Enabled: true}, @@ -95,7 +87,7 @@ func TestApplyWithMultipleJobs(t *testing.T) { }, }, } - d := m.Apply(context.Background(), b) + d := bundle.Apply(context.Background(), b, DefaultQueueing()) assert.Len(t, d, 0) assert.False(t, b.Config.Resources.Jobs["job1"].Queue.Enabled) assert.True(t, b.Config.Resources.Jobs["job2"].Queue.Enabled) From 560fc41fbbf8e2d41f4450a5c19d55b3d1529851 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 22 Apr 2024 12:29:58 +0200 Subject: [PATCH 3/7] Update bundle/config/mutator/default_queueing_test.go --- bundle/config/mutator/default_queueing_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/config/mutator/default_queueing_test.go b/bundle/config/mutator/default_queueing_test.go index 26b98328b4..aecbed47ab 100644 --- a/bundle/config/mutator/default_queueing_test.go +++ b/bundle/config/mutator/default_queueing_test.go @@ -16,7 +16,7 @@ func TestDefaultQueueing(t *testing.T) { assert.IsType(t, &defaultQueueing{}, m) } -func TestName(t *testing.T) { +func TestDefaultQueueingName(t *testing.T) { m := DefaultQueueing() assert.Equal(t, "DefaultQueueing", m.Name()) } From 785a5e398f1aa22851909940b92d9b211a500f22 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 22 Apr 2024 12:30:04 +0200 Subject: [PATCH 4/7] Update bundle/config/mutator/default_queueing_test.go --- bundle/config/mutator/default_queueing_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/config/mutator/default_queueing_test.go b/bundle/config/mutator/default_queueing_test.go index aecbed47ab..bc87b3e2ea 100644 --- a/bundle/config/mutator/default_queueing_test.go +++ b/bundle/config/mutator/default_queueing_test.go @@ -21,7 +21,7 @@ func TestDefaultQueueingName(t *testing.T) { assert.Equal(t, "DefaultQueueing", m.Name()) } -func TestApplyNoJobs(t *testing.T) { +func TestDefaultQueueingApplyNoJobs(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ Resources: config.Resources{}, From 37f8e7e207f32ac77ea1a01603c6dd7059c8fb21 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 22 Apr 2024 12:30:13 +0200 Subject: [PATCH 5/7] Update bundle/config/mutator/default_queueing_test.go --- bundle/config/mutator/default_queueing_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/config/mutator/default_queueing_test.go b/bundle/config/mutator/default_queueing_test.go index bc87b3e2ea..7537c3cffd 100644 --- a/bundle/config/mutator/default_queueing_test.go +++ b/bundle/config/mutator/default_queueing_test.go @@ -32,7 +32,7 @@ func TestDefaultQueueingApplyNoJobs(t *testing.T) { assert.Len(t, b.Config.Resources.Jobs, 0) } -func TestApplyJobsAlreadyEnabled(t *testing.T) { +func TestDefaultQueueingApplyJobsAlreadyEnabled(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ Resources: config.Resources{ From 34b0699f54d9c864e9cf59e47c586940c0077aed Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 22 Apr 2024 12:30:18 +0200 Subject: [PATCH 6/7] Update bundle/config/mutator/default_queueing_test.go --- bundle/config/mutator/default_queueing_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/config/mutator/default_queueing_test.go b/bundle/config/mutator/default_queueing_test.go index 7537c3cffd..45f892208a 100644 --- a/bundle/config/mutator/default_queueing_test.go +++ b/bundle/config/mutator/default_queueing_test.go @@ -51,7 +51,7 @@ func TestDefaultQueueingApplyJobsAlreadyEnabled(t *testing.T) { assert.True(t, b.Config.Resources.Jobs["job"].Queue.Enabled) } -func TestApplyEnableQueueing(t *testing.T) { +func TestDefaultQueueingApplyEnableQueueing(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ Resources: config.Resources{ From 27dee9c1bf5b4eb32220d1bef6fa09c4dc5944bc Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 22 Apr 2024 12:30:22 +0200 Subject: [PATCH 7/7] Update bundle/config/mutator/default_queueing_test.go --- bundle/config/mutator/default_queueing_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/config/mutator/default_queueing_test.go b/bundle/config/mutator/default_queueing_test.go index 45f892208a..ea60daf7f1 100644 --- a/bundle/config/mutator/default_queueing_test.go +++ b/bundle/config/mutator/default_queueing_test.go @@ -67,7 +67,7 @@ func TestDefaultQueueingApplyEnableQueueing(t *testing.T) { assert.True(t, b.Config.Resources.Jobs["job"].Queue.Enabled) } -func TestApplyWithMultipleJobs(t *testing.T) { +func TestDefaultQueueingApplyWithMultipleJobs(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ Resources: config.Resources{