-
Notifications
You must be signed in to change notification settings - Fork 168
Add support for "debug runs" #493
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
48d6df3
Add support for "debug runs"
lennartkats-db 27b2b77
Introduce DATABRICKS_COMPUTE environment variable
lennartkats-db 823c868
Increase max concurrent runs for debug runs
lennartkats-db 6414701
Merge remote-tracking branch 'upstream/main' into add-debug-runs
lennartkats-db bdd2124
Rename Deploy() to deploy()
lennartkats-db 7c654ec
Pause schedule for debug jobs
lennartkats-db b7a80af
Merge remote-tracking branch 'upstream/main' into add-debug-runs
lennartkats-db File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| package mutator | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
|
|
||
| "github.com/databricks/cli/bundle" | ||
| "github.com/databricks/cli/bundle/config" | ||
| "github.com/databricks/cli/bundle/config/resources" | ||
| ) | ||
|
|
||
| type overrideCompute struct { | ||
| compute string | ||
| } | ||
|
|
||
| func OverrideCompute(compute string) bundle.Mutator { | ||
| return &overrideCompute{compute: compute} | ||
| } | ||
|
|
||
| func (m *overrideCompute) Name() string { | ||
| return "OverrideCompute" | ||
| } | ||
|
|
||
| func (m *overrideCompute) overrideJobCompute(j *resources.Job) { | ||
| for i := range j.Tasks { | ||
| task := &j.Tasks[i] | ||
| if task.NewCluster != nil { | ||
| task.NewCluster = nil | ||
| task.ExistingClusterId = m.compute | ||
| } else if task.ExistingClusterId != "" { | ||
| task.ExistingClusterId = m.compute | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func (m *overrideCompute) Apply(ctx context.Context, b *bundle.Bundle) error { | ||
| if m.compute == "" { | ||
| return nil | ||
| } | ||
| if b.Config.Bundle.Mode != config.Debug { | ||
| return fmt.Errorf("cannot override compute for an environment that does not use 'mode: debug'") | ||
| } | ||
|
|
||
| r := b.Config.Resources | ||
| for i := range r.Jobs { | ||
| m.overrideJobCompute(r.Jobs[i]) | ||
| } | ||
|
|
||
| return nil | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| package mutator_test | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| "github.com/databricks/cli/bundle" | ||
| "github.com/databricks/cli/bundle/config" | ||
| "github.com/databricks/cli/bundle/config/mutator" | ||
| "github.com/databricks/cli/bundle/config/resources" | ||
| "github.com/databricks/databricks-sdk-go/service/compute" | ||
| "github.com/databricks/databricks-sdk-go/service/jobs" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestOverrideCompute(t *testing.T) { | ||
| bundle := &bundle.Bundle{ | ||
| Config: config.Root{ | ||
| Bundle: config.Bundle{ | ||
| Mode: config.Debug, | ||
| }, | ||
| Resources: config.Resources{ | ||
| Jobs: map[string]*resources.Job{ | ||
| "job1": {JobSettings: &jobs.JobSettings{ | ||
| Name: "job1", | ||
| Tasks: []jobs.JobTaskSettings{ | ||
| { | ||
| NewCluster: &compute.BaseClusterInfo{}, | ||
| }, | ||
| { | ||
| ExistingClusterId: "cluster2", | ||
| }, | ||
| }, | ||
| }}, | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| m := mutator.OverrideCompute("newClusterID") | ||
| err := m.Apply(context.Background(), bundle) | ||
| require.NoError(t, err) | ||
| assert.Nil(t, bundle.Config.Resources.Jobs["job1"].Tasks[0].NewCluster) | ||
| assert.Equal(t, "newClusterID", bundle.Config.Resources.Jobs["job1"].Tasks[0].ExistingClusterId) | ||
| assert.Equal(t, "newClusterID", bundle.Config.Resources.Jobs["job1"].Tasks[1].ExistingClusterId) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| package mutator | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
|
|
||
| "github.com/databricks/cli/bundle" | ||
| "github.com/databricks/cli/bundle/config" | ||
| "github.com/databricks/databricks-sdk-go/service/ml" | ||
| ) | ||
|
|
||
| type processEnvironmentMode struct{} | ||
|
|
||
| const debugConcurrentRuns = 4 | ||
|
|
||
| func ProcessEnvironmentMode() bundle.Mutator { | ||
| return &processEnvironmentMode{} | ||
| } | ||
|
|
||
| func (m *processEnvironmentMode) Name() string { | ||
| return "ProcessEnvironmentMode" | ||
| } | ||
|
|
||
| // Mark all resources as being for 'debug' purposes, i.e. | ||
| // changing their their name, adding tags, and (in the future) | ||
| // marking them as 'hidden' in the UI. | ||
| func processDebugMode(b *bundle.Bundle) error { | ||
| r := b.Config.Resources | ||
|
|
||
| for i := range r.Jobs { | ||
| r.Jobs[i].Name = "[debug] " + r.Jobs[i].Name | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Saad: this should probably say [username debug]
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| if r.Jobs[i].Tags == nil { | ||
| r.Jobs[i].Tags = make(map[string]string) | ||
| } | ||
| r.Jobs[i].Tags["debug"] = "" | ||
| if r.Jobs[i].MaxConcurrentRuns == 0 { | ||
| r.Jobs[i].MaxConcurrentRuns = debugConcurrentRuns | ||
| } | ||
|
lennartkats-db marked this conversation as resolved.
|
||
| if r.Jobs[i].Schedule != nil { | ||
| r.Jobs[i].Schedule.PauseStatus = "PAUSED" | ||
| } | ||
| if r.Jobs[i].Continuous != nil { | ||
| r.Jobs[i].Continuous.PauseStatus = "PAUSED" | ||
| } | ||
| if r.Jobs[i].Trigger != nil { | ||
| r.Jobs[i].Trigger.PauseStatus = "PAUSED" | ||
| } | ||
| } | ||
|
|
||
| for i := range r.Pipelines { | ||
| r.Pipelines[i].Name = "[debug] " + r.Pipelines[i].Name | ||
| r.Pipelines[i].Development = true | ||
| // (pipelines don't yet support tags) | ||
| } | ||
|
|
||
| for i := range r.Models { | ||
| r.Models[i].Name = "[debug] " + r.Models[i].Name | ||
| r.Models[i].Tags = append(r.Models[i].Tags, ml.ModelTag{Key: "debug", Value: ""}) | ||
| } | ||
|
|
||
| for i := range r.Experiments { | ||
| r.Experiments[i].Name = "[debug] " + r.Experiments[i].Name | ||
| r.Experiments[i].Tags = append(r.Experiments[i].Tags, ml.ExperimentTag{Key: "debug", Value: ""}) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (m *processEnvironmentMode) Apply(ctx context.Context, b *bundle.Bundle) error { | ||
| switch b.Config.Bundle.Mode { | ||
| case config.Debug: | ||
| return processDebugMode(b) | ||
| case config.Default, "": | ||
| // No action | ||
| case config.PullRequest: | ||
| return fmt.Errorf("not implemented") | ||
| default: | ||
| return fmt.Errorf("unsupported value specified for 'mode': %s", b.Config.Bundle.Mode) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| package mutator_test | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| "github.com/databricks/cli/bundle" | ||
| "github.com/databricks/cli/bundle/config" | ||
| "github.com/databricks/cli/bundle/config/mutator" | ||
| "github.com/databricks/cli/bundle/config/resources" | ||
| "github.com/databricks/databricks-sdk-go/service/jobs" | ||
| "github.com/databricks/databricks-sdk-go/service/ml" | ||
| "github.com/databricks/databricks-sdk-go/service/pipelines" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestProcessEnvironmentModeApplyDebug(t *testing.T) { | ||
| bundle := &bundle.Bundle{ | ||
| Config: config.Root{ | ||
| Bundle: config.Bundle{ | ||
| Mode: config.Debug, | ||
| }, | ||
| Resources: config.Resources{ | ||
| Jobs: map[string]*resources.Job{ | ||
| "job1": {JobSettings: &jobs.JobSettings{Name: "job1"}}, | ||
| }, | ||
| Pipelines: map[string]*resources.Pipeline{ | ||
| "pipeline1": {PipelineSpec: &pipelines.PipelineSpec{Name: "pipeline1"}}, | ||
| }, | ||
| Experiments: map[string]*resources.MlflowExperiment{ | ||
| "experiment1": {Experiment: &ml.Experiment{Name: "experiment1"}}, | ||
| }, | ||
| Models: map[string]*resources.MlflowModel{ | ||
| "model1": {Model: &ml.Model{Name: "model1"}}, | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| m := mutator.ProcessEnvironmentMode() | ||
| err := m.Apply(context.Background(), bundle) | ||
| require.NoError(t, err) | ||
| assert.Equal(t, "[debug] job1", bundle.Config.Resources.Jobs["job1"].Name) | ||
| assert.Equal(t, "[debug] pipeline1", bundle.Config.Resources.Pipelines["pipeline1"].Name) | ||
| assert.Equal(t, "[debug] experiment1", bundle.Config.Resources.Experiments["experiment1"].Name) | ||
| assert.Equal(t, "[debug] model1", bundle.Config.Resources.Models["model1"].Name) | ||
| assert.Equal(t, "debug", bundle.Config.Resources.Experiments["experiment1"].Experiment.Tags[0].Key) | ||
| assert.True(t, bundle.Config.Resources.Pipelines["pipeline1"].PipelineSpec.Development) | ||
| } | ||
|
|
||
| func TestProcessEnvironmentModeApplyDefault(t *testing.T) { | ||
| bundle := &bundle.Bundle{ | ||
| Config: config.Root{ | ||
| Bundle: config.Bundle{ | ||
| Mode: config.Default, | ||
| }, | ||
| Resources: config.Resources{ | ||
| Jobs: map[string]*resources.Job{ | ||
| "job1": {JobSettings: &jobs.JobSettings{Name: "job1"}}, | ||
| }, | ||
| Pipelines: map[string]*resources.Pipeline{ | ||
| "pipeline1": {PipelineSpec: &pipelines.PipelineSpec{Name: "pipeline1"}}, | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| m := mutator.ProcessEnvironmentMode() | ||
| err := m.Apply(context.Background(), bundle) | ||
| require.NoError(t, err) | ||
| assert.Equal(t, "job1", bundle.Config.Resources.Jobs["job1"].Name) | ||
| assert.Equal(t, "pipeline1", bundle.Config.Resources.Pipelines["pipeline1"].Name) | ||
| assert.False(t, bundle.Config.Resources.Pipelines["pipeline1"].PipelineSpec.Development) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ resources: | |
|
|
||
| environments: | ||
| development: | ||
| mode: debug | ||
| resources: | ||
| pipelines: | ||
| nyc_taxi_pipeline: | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should rather be a property of a bundle flipped by a command-line flag and not the environment variable.
https://github.com/databricks/cli/blob/ccbcccd92903bc0a6dbdbc94072035b30aca8527/cmd/bundle/destroy.go#L27C1-L28
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thing is, it's not really "ephemeral" at this time. We need to create real, persistent jobs and pipelines. The main reason is that one job may need to reference another job or another pipeline in the same DAB.
When we say "ephemeral" in this case we really just mean that we need to have "some" garbage collection mechanism that removes these persistent jobs after perhaps 24 hours of non-use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can add that cleanup capability for previous normal runs from bundle runner 🤷♂️