Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 215
Source-linked deployments for bundles in the workspace#1884
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
6b22fdeae26beafbb9be9086dfbca43f7dd5a221516c12308b2164c00004ed226f245351c8ef54cf69298cd95ebe9b7289a3c6d5780ea3a0e7165ec49f6bc9e65b50c53e1f6d00bb683e8825d51d7b27e55f715d518aa14aeb98138c8fb35234d9715d090708ee8de55d04569530a2a9aba35aeddb68a7eede5226a036d166bf5f3File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -6,6 +6,7 @@ import ( | ||
| "github.com/databricks/cli/bundle" | ||
| "github.com/databricks/cli/bundle/config" | ||
| "github.com/databricks/cli/libs/dbr" | ||
| "github.com/databricks/cli/libs/diag" | ||
| "github.com/databricks/cli/libs/dyn" | ||
| "github.com/databricks/cli/libs/iamutil" | ||
| @@ -57,6 +58,14 @@ func transformDevelopmentMode(ctx context.Context, b *bundle.Bundle) { | ||
| t.TriggerPauseStatus = config.Paused | ||
| } | ||
| if !config.IsExplicitlyDisabled(t.SourceLinkedDeployment) { | ||
| isInWorkspace := strings.HasPrefix(b.SyncRootPath, "/Workspace/") | ||
| if isInWorkspace && dbr.RunsOnRuntime(ctx) { | ||
| enabled := true | ||
| t.SourceLinkedDeployment = &enabled | ||
| } | ||
| } | ||
pietern marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| if !config.IsExplicitlyDisabled(t.PipelinesDevelopment) { | ||
| enabled := true | ||
| t.PipelinesDevelopment = &enabled | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -3,14 +3,17 @@ package mutator | ||
| import ( | ||
| "context" | ||
| "reflect" | ||
| "runtime" | ||
| "strings" | ||
| "testing" | ||
| "github.com/databricks/cli/bundle" | ||
| "github.com/databricks/cli/bundle/config" | ||
| "github.com/databricks/cli/bundle/config/resources" | ||
| "github.com/databricks/cli/libs/dbr" | ||
| "github.com/databricks/cli/libs/diag" | ||
| "github.com/databricks/cli/libs/tags" | ||
| "github.com/databricks/cli/libs/vfs" | ||
| sdkconfig "github.com/databricks/databricks-sdk-go/config" | ||
| "github.com/databricks/databricks-sdk-go/service/catalog" | ||
| "github.com/databricks/databricks-sdk-go/service/compute" | ||
| @@ -133,6 +136,7 @@ func mockBundle(mode config.Mode) *bundle.Bundle { | ||
| }, | ||
| }, | ||
| }, | ||
| SyncRoot: vfs.MustNew("/Users/lennart.kats@databricks.com"), | ||
Contributor 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. This probably fails on Windows.
| ||
| // Use AWS implementation for testing. | ||
| Tagging: tags.ForCloud(&sdkconfig.Config{ | ||
| Host: "https://company.cloud.databricks.com", | ||
| @@ -515,3 +519,32 @@ func TestPipelinesDevelopmentDisabled(t *testing.T) { | ||
| assert.False(t, b.Config.Resources.Pipelines["pipeline1"].PipelineSpec.Development) | ||
| } | ||
| func TestSourceLinkedDeploymentEnabled(t *testing.T) { | ||
| b, diags := processSourceLinkedBundle(t, true) | ||
| require.NoError(t, diags.Error()) | ||
| assert.True(t, *b.Config.Presets.SourceLinkedDeployment) | ||
| } | ||
| func TestSourceLinkedDeploymentDisabled(t *testing.T) { | ||
| b, diags := processSourceLinkedBundle(t, false) | ||
| require.NoError(t, diags.Error()) | ||
| assert.False(t, *b.Config.Presets.SourceLinkedDeployment) | ||
| } | ||
| func processSourceLinkedBundle(t *testing.T, presetEnabled bool) (*bundle.Bundle, diag.Diagnostics) { | ||
| if runtime.GOOS == "windows" { | ||
| t.Skip("this test is not applicable on Windows because source-linked mode works only in the Databricks Workspace") | ||
| } | ||
| b := mockBundle(config.Development) | ||
| workspacePath := "/Workspace/lennart@company.com/" | ||
| b.SyncRootPath = workspacePath | ||
| b.Config.Presets.SourceLinkedDeployment = &presetEnabled | ||
| ctx := dbr.MockRuntime(context.Background(), true) | ||
| m := bundle.Seq(ProcessTargetMode(), ApplyPresets()) | ||
| diags := bundle.Apply(ctx, b, m) | ||
| return b, diags | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
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.
Warnings should include position/file/line information where possible.
This condition triggers only if the user explicitly configured the setting, so we should have this information. You can call
b.Config.GetLocations()with the path to the setting to get these.Not blocking for this PR.
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 have this setting in
targets.[target].presets.source_linked_deploymentand this should be in the config indeed in that case, but we have nil inb.Config.Targetswhich makes location lookup not possibleIt is assigned here
cli/bundle/config/mutator/select_target.go
Line 53 in ed19466
I can try to remove this nil assingment from there, not sure why it is needed. If I remove this line locations are available
Also we can show something like this without locations

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.
You should just be able to use the locations from top-level
presets.source_linked_deployment. That should include the target override locations as well IIRC.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.
That works indeed, thanks!
Will make another PR later