Skip to content
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

Refactor bundle init #2074

Open
wants to merge 36 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
070ae2f
Refactor `bundle init`
shreyas-goenka Jan 3, 2025
daa2b91
undo test changes
shreyas-goenka Jan 3, 2025
a743139
remove final telemetry bits
shreyas-goenka Jan 3, 2025
d238dd8
add resolver
shreyas-goenka Jan 3, 2025
2965c30
add unit tests for reader
shreyas-goenka Jan 3, 2025
ed24f28
more test
shreyas-goenka Jan 3, 2025
af89580
fix tests and lint
shreyas-goenka Jan 6, 2025
9f83fe0
fix test
shreyas-goenka Jan 6, 2025
00266cb
fix another bug
shreyas-goenka Jan 6, 2025
2b1c5cb
skip test on windows
shreyas-goenka Jan 6, 2025
8607e08
add unit tests for resolver
shreyas-goenka Jan 6, 2025
39dfe05
add test for the get method
shreyas-goenka Jan 6, 2025
7105777
-
shreyas-goenka Jan 6, 2025
4755db4
custom rename to avoid break
shreyas-goenka Jan 6, 2025
0a68fb3
-
shreyas-goenka Jan 6, 2025
8202aaf
more test
shreyas-goenka Jan 6, 2025
4743095
lint
shreyas-goenka Jan 6, 2025
57a7519
cleanup a bit
shreyas-goenka Jan 6, 2025
38d47e6
fix aliasing
shreyas-goenka Jan 6, 2025
ad28ce1
fix aliasing
shreyas-goenka Jan 6, 2025
f394606
Merge remote-tracking branch 'origin' into refactor-bundle-init-squashed
shreyas-goenka Jan 6, 2025
2a0dbde
remove fail reader
shreyas-goenka Jan 9, 2025
a2a3ae7
address comments
shreyas-goenka Jan 9, 2025
30d683a
merge
shreyas-goenka Jan 9, 2025
0f62d0e
Merge remote-tracking branch 'origin' into refactor-bundle-init-squashed
shreyas-goenka Jan 16, 2025
44586ff
address comments
shreyas-goenka Jan 16, 2025
9bf0cac
use constants
shreyas-goenka Jan 17, 2025
eb6fc09
fmt
shreyas-goenka Jan 17, 2025
f7557bf
simplify the custom template code
shreyas-goenka Jan 17, 2025
34f02c8
fix test
shreyas-goenka Jan 17, 2025
35c0949
fix tests
shreyas-goenka Jan 17, 2025
7f9eb32
fix test on windows
shreyas-goenka Jan 17, 2025
7800f34
use mock function
shreyas-goenka Jan 17, 2025
9649f23
fix
shreyas-goenka Jan 17, 2025
56e4d9f
fmt
shreyas-goenka Jan 17, 2025
6e11778
Merge remote-tracking branch 'origin' into refactor-bundle-init-squashed
shreyas-goenka Jan 17, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions bundle/run/app_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package run

import (
"bytes"
"context"
"errors"
"os"
Expand All @@ -16,7 +15,6 @@ import (
"github.com/databricks/cli/bundle/internal/bundletest"
"github.com/databricks/cli/libs/cmdio"
"github.com/databricks/cli/libs/dyn"
"github.com/databricks/cli/libs/flags"
"github.com/databricks/cli/libs/vfs"
"github.com/databricks/databricks-sdk-go/experimental/mocks"
"github.com/databricks/databricks-sdk-go/service/apps"
Expand Down Expand Up @@ -76,9 +74,7 @@ func setupBundle(t *testing.T) (context.Context, *bundle.Bundle, *mocks.MockWork
b.SetWorkpaceClient(mwc.WorkspaceClient)
bundletest.SetLocation(b, "resources.apps.my_app", []dyn.Location{{File: "./databricks.yml"}})

ctx := context.Background()
ctx = cmdio.InContext(ctx, cmdio.NewIO(ctx, flags.OutputText, &bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{}, "", "..."))
ctx = cmdio.NewContext(ctx, cmdio.NewLogger(flags.ModeAppend))
ctx := cmdio.MockDiscard(context.Background())

diags := bundle.Apply(ctx, b, bundle.Seq(
mutator.DefineDefaultWorkspacePaths(),
Expand Down
9 changes: 4 additions & 5 deletions bundle/run/job_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package run

import (
"bytes"
"context"
"testing"
"time"
Expand Down Expand Up @@ -159,8 +158,8 @@ func TestJobRunnerRestart(t *testing.T) {

m := mocks.NewMockWorkspaceClient(t)
b.SetWorkpaceClient(m.WorkspaceClient)
ctx := context.Background()
ctx = cmdio.InContext(ctx, cmdio.NewIO(ctx, flags.OutputText, &bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{}, "", ""))

ctx := cmdio.MockDiscard(context.Background())
ctx = cmdio.NewContext(ctx, cmdio.NewLogger(flags.ModeAppend))

jobApi := m.GetMockJobsAPI()
Expand Down Expand Up @@ -230,8 +229,8 @@ func TestJobRunnerRestartForContinuousUnpausedJobs(t *testing.T) {

m := mocks.NewMockWorkspaceClient(t)
b.SetWorkpaceClient(m.WorkspaceClient)
ctx := context.Background()
ctx = cmdio.InContext(ctx, cmdio.NewIO(ctx, flags.OutputText, &bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{}, "", "..."))

ctx := cmdio.MockDiscard(context.Background())
ctx = cmdio.NewContext(ctx, cmdio.NewLogger(flags.ModeAppend))

jobApi := m.GetMockJobsAPI()
Expand Down
5 changes: 2 additions & 3 deletions bundle/run/pipeline_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package run

import (
"bytes"
"context"
"testing"
"time"
Expand Down Expand Up @@ -75,8 +74,8 @@ func TestPipelineRunnerRestart(t *testing.T) {
Host: "https://test.com",
}
b.SetWorkpaceClient(m.WorkspaceClient)
ctx := context.Background()
ctx = cmdio.InContext(ctx, cmdio.NewIO(ctx, flags.OutputText, &bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{}, "", "..."))

ctx := cmdio.MockDiscard(context.Background())
ctx = cmdio.NewContext(ctx, cmdio.NewLogger(flags.ModeAppend))

mockWait := &pipelines.WaitGetPipelineIdle[struct{}]{
Expand Down
246 changes: 15 additions & 231 deletions cmd/bundle/init.go
Original file line number Diff line number Diff line change
@@ -1,175 +1,15 @@
package bundle

import (
"context"
"errors"
"fmt"
"io/fs"
"os"
"path/filepath"
"slices"
"strings"

"github.com/databricks/cli/cmd/root"
"github.com/databricks/cli/libs/cmdio"
"github.com/databricks/cli/libs/dbr"
"github.com/databricks/cli/libs/filer"
"github.com/databricks/cli/libs/git"
"github.com/databricks/cli/libs/template"
"github.com/spf13/cobra"
)

var gitUrlPrefixes = []string{
"https://",
"git@",
}

type nativeTemplate struct {
name string
gitUrl string
description string
aliases []string
hidden bool
}

const customTemplate = "custom..."

var nativeTemplates = []nativeTemplate{
{
name: "default-python",
description: "The default Python template for Notebooks / Delta Live Tables / Workflows",
},
{
name: "default-sql",
description: "The default SQL template for .sql files that run with Databricks SQL",
},
{
name: "dbt-sql",
description: "The dbt SQL template (databricks.com/blog/delivering-cost-effective-data-real-time-dbt-and-databricks)",
},
{
name: "mlops-stacks",
gitUrl: "https://github.com/databricks/mlops-stacks",
description: "The Databricks MLOps Stacks template (github.com/databricks/mlops-stacks)",
aliases: []string{"mlops-stack"},
},
{
name: "default-pydabs",
gitUrl: "https://databricks.github.io/workflows-authoring-toolkit/pydabs-template.git",
hidden: true,
description: "The default PyDABs template",
},
{
name: customTemplate,
description: "Bring your own template",
},
}

// Return template descriptions for command-line help
func nativeTemplateHelpDescriptions() string {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved totemplate.HelpDescriptions()

var lines []string
for _, template := range nativeTemplates {
if template.name != customTemplate && !template.hidden {
lines = append(lines, fmt.Sprintf("- %s: %s", template.name, template.description))
}
}
return strings.Join(lines, "\n")
}

// Return template options for an interactive prompt
func nativeTemplateOptions() []cmdio.Tuple {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to template.options()

names := make([]cmdio.Tuple, 0, len(nativeTemplates))
for _, template := range nativeTemplates {
if template.hidden {
continue
}
tuple := cmdio.Tuple{
Name: template.name,
Id: template.description,
}
names = append(names, tuple)
}
return names
}

func getNativeTemplateByDescription(description string) string {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved inline to template. SelectTemplate()

for _, template := range nativeTemplates {
if template.description == description {
return template.name
}
}
return ""
}

func getUrlForNativeTemplate(name string) string {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced by template.Get()

for _, template := range nativeTemplates {
if template.name == name {
return template.gitUrl
}
if slices.Contains(template.aliases, name) {
return template.gitUrl
}
}
return ""
}

func getFsForNativeTemplate(name string) (fs.FS, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced by template.Reader.FS() method.

builtin, err := template.Builtin()
if err != nil {
return nil, err
}

// If this is a built-in template, the return value will be non-nil.
var templateFS fs.FS
for _, entry := range builtin {
if entry.Name == name {
templateFS = entry.FS
break
}
}

return templateFS, nil
}

func isRepoUrl(url string) bool {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to template.isRepoUrl

result := false
for _, prefix := range gitUrlPrefixes {
if strings.HasPrefix(url, prefix) {
result = true
break
}
}
return result
}

// Computes the repo name from the repo URL. Treats the last non empty word
// when splitting at '/' as the repo name. For example: for url git@github.com:databricks/cli.git
// the name would be "cli.git"
func repoName(url string) string {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to template.repoName in reader.go

parts := strings.Split(strings.TrimRight(url, "/"), "/")
return parts[len(parts)-1]
}

func constructOutputFiler(ctx context.Context, outputDir string) (filer.Filer, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to template.constructOutputFiler. Also added unit tests for it in TestDefaultWriterConfigure and TestDefaultWriterConfigureOnDBR

outputDir, err := filepath.Abs(outputDir)
if err != nil {
return nil, err
}

// If the CLI is running on DBR and we're writing to the workspace file system,
// use the extension-aware workspace filesystem filer to instantiate the template.
//
// It is not possible to write notebooks through the workspace filesystem's FUSE mount.
// Therefore this is the only way we can initialize templates that contain notebooks
// when running the CLI on DBR and initializing a template to the workspace.
//
if strings.HasPrefix(outputDir, "/Workspace/") && dbr.RunsOnRuntime(ctx) {
return filer.NewWorkspaceFilesExtensionsClient(root.WorkspaceClient(ctx), outputDir)
}

return filer.NewLocalClient(outputDir)
}

func newInitCommand() *cobra.Command {
cmd := &cobra.Command{
Use: "init [TEMPLATE_PATH]",
Expand All @@ -182,7 +22,7 @@ TEMPLATE_PATH optionally specifies which template to use. It can be one of the f
- a local file system path with a template directory
- a Git repository URL, e.g. https://github.com/my/repository

See https://docs.databricks.com/en/dev-tools/bundles/templates.html for more information on templates.`, nativeTemplateHelpDescriptions()),
See https://docs.databricks.com/en/dev-tools/bundles/templates.html for more information on templates.`, template.HelpDescriptions()),
}

var configFile string
Expand All @@ -202,88 +42,32 @@ See https://docs.databricks.com/en/dev-tools/bundles/templates.html for more inf
return errors.New("only one of --tag or --branch can be specified")
}

// Git ref to use for template initialization
ref := branch
if tag != "" {
ref = tag
}

ctx := cmd.Context()
var templatePath string
var templatePathOrUrl string
if len(args) > 0 {
templatePath = args[0]
} else {
var err error
if !cmdio.IsPromptSupported(ctx) {
return errors.New("please specify a template")
}
description, err := cmdio.SelectOrdered(ctx, nativeTemplateOptions(), "Template to use")
if err != nil {
return err
}
templatePath = getNativeTemplateByDescription(description)
templatePathOrUrl = args[0]
}

outputFiler, err := constructOutputFiler(ctx, outputDir)
if err != nil {
return err
r := template.Resolver{
TemplatePathOrUrl: templatePathOrUrl,
ConfigFile: configFile,
OutputDir: outputDir,
TemplateDir: templateDir,
Tag: tag,
Branch: branch,
}

if templatePath == customTemplate {
ctx := cmd.Context()
tmpl, err := r.Resolve(ctx)
if errors.Is(err, template.ErrCustomSelected) {
cmdio.LogString(ctx, "Please specify a path or Git repository to use a custom template.")
cmdio.LogString(ctx, "See https://docs.databricks.com/en/dev-tools/bundles/templates.html to learn more about custom templates.")
return nil
}

// Expand templatePath to a git URL if it's an alias for a known native template
// and we know it's git URL.
if gitUrl := getUrlForNativeTemplate(templatePath); gitUrl != "" {
templatePath = gitUrl
}

if !isRepoUrl(templatePath) {
if templateDir != "" {
return errors.New("--template-dir can only be used with a Git repository URL")
}

templateFS, err := getFsForNativeTemplate(templatePath)
if err != nil {
return err
}

// If this is not a built-in template, then it must be a local file system path.
if templateFS == nil {
templateFS = os.DirFS(templatePath)
}

// skip downloading the repo because input arg is not a URL. We assume
// it's a path on the local file system in that case
return template.Materialize(ctx, configFile, templateFS, outputFiler)
}

// Create a temporary directory with the name of the repository. The '*'
// character is replaced by a random string in the generated temporary directory.
repoDir, err := os.MkdirTemp("", repoName(templatePath)+"-*")
if err != nil {
return err
}

// start the spinner
promptSpinner := cmdio.Spinner(ctx)
promptSpinner <- "Downloading the template\n"

// TODO: Add automated test that the downloaded git repo is cleaned up.
// Clone the repository in the temporary directory
err = git.Clone(ctx, templatePath, ref, repoDir)
close(promptSpinner)
if err != nil {
return err
}
defer tmpl.Reader.Cleanup()

// Clean up downloaded repository once the template is materialized.
defer os.RemoveAll(repoDir)
templateFS := os.DirFS(filepath.Join(repoDir, templateDir))
return template.Materialize(ctx, configFile, templateFS, outputFiler)
return tmpl.Writer.Materialize(ctx, tmpl.Reader)
}
return cmd
}
14 changes: 11 additions & 3 deletions integration/bundle/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"github.com/databricks/cli/internal/testutil"
"github.com/databricks/cli/libs/cmdio"
"github.com/databricks/cli/libs/env"
"github.com/databricks/cli/libs/filer"
"github.com/databricks/cli/libs/flags"
"github.com/databricks/cli/libs/folders"
"github.com/databricks/cli/libs/template"
Expand All @@ -40,10 +39,19 @@ func initTestTemplateWithBundleRoot(t testutil.TestingT, ctx context.Context, te
cmd := cmdio.NewIO(ctx, flags.OutputJSON, strings.NewReader(""), os.Stdout, os.Stderr, "", "bundles")
ctx = cmdio.InContext(ctx, cmd)

out, err := filer.NewLocalClient(bundleRoot)
r := template.Resolver{
TemplatePathOrUrl: templateRoot,
ConfigFile: configFilePath,
OutputDir: bundleRoot,
}

tmpl, err := r.Resolve(ctx)
require.NoError(t, err)
err = template.Materialize(ctx, configFilePath, os.DirFS(templateRoot), out)
defer tmpl.Reader.Cleanup()

err = tmpl.Writer.Materialize(ctx, tmpl.Reader)
require.NoError(t, err)

return bundleRoot
}

Expand Down
Loading
Loading