Skip to content

Commit

Permalink
Use fs.FS interface to read template (#1910)
Browse files Browse the repository at this point in the history
## Changes

While working on the v2 of #1744, I found that:
* Template initialization first copies built-in templates to a temporary
directory before initializing them
* Reading a template's contents goes through a `filer.Filer` but is
hardcoded to a local one

This change updates the interface for reading templates to be `fs.FS`.
This is compatible with the `embed.FS` type for the built-in templates,
so they no longer have to be copied to a temporary directory before
being used.

The alternative is to use a `filer.Filer` throughout, but this would
have required even more plumbing, and we don't need to _read_ templates,
including notebooks, from the workspace filesystem (yet?).

As part of making `template.Materialize` take an `fs.FS` argument, the
logic to match a given argument to a particular built-in template in the
`init` command has moved to sit next to its implementation.

## Tests

Existing tests pass.
  • Loading branch information
pietern authored Nov 20, 2024
1 parent 72dde79 commit 4fea021
Show file tree
Hide file tree
Showing 15 changed files with 232 additions and 179 deletions.
35 changes: 33 additions & 2 deletions cmd/bundle/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package bundle
import (
"errors"
"fmt"
"io/fs"
"os"
"path/filepath"
"slices"
Expand Down Expand Up @@ -109,6 +110,24 @@ func getUrlForNativeTemplate(name string) string {
return ""
}

func getFsForNativeTemplate(name string) (fs.FS, error) {
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 {
result := false
for _, prefix := range gitUrlPrefixes {
Expand Down Expand Up @@ -198,9 +217,20 @@ See https://docs.databricks.com/en/dev-tools/bundles/templates.html for more inf
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, templatePath, outputDir)
return template.Materialize(ctx, configFile, templateFS, outputDir)
}

// Create a temporary directory with the name of the repository. The '*'
Expand All @@ -224,7 +254,8 @@ See https://docs.databricks.com/en/dev-tools/bundles/templates.html for more inf

// Clean up downloaded repository once the template is materialized.
defer os.RemoveAll(repoDir)
return template.Materialize(ctx, configFile, filepath.Join(repoDir, templateDir), outputDir)
templateFS := os.DirFS(filepath.Join(repoDir, templateDir))
return template.Materialize(ctx, configFile, templateFS, outputDir)
}
return cmd
}
2 changes: 1 addition & 1 deletion internal/bundle/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func initTestTemplateWithBundleRoot(t *testing.T, ctx context.Context, templateN
cmd := cmdio.NewIO(flags.OutputJSON, strings.NewReader(""), os.Stdout, os.Stderr, "", "bundles")
ctx = cmdio.InContext(ctx, cmd)

err = template.Materialize(ctx, configFilePath, templateRoot, bundleRoot)
err = template.Materialize(ctx, configFilePath, os.DirFS(templateRoot), bundleRoot)
return bundleRoot, err
}

Expand Down
9 changes: 8 additions & 1 deletion libs/jsonschema/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ package jsonschema
import (
"encoding/json"
"fmt"
"io/fs"
"os"
"path/filepath"
"regexp"
"slices"

Expand Down Expand Up @@ -255,7 +257,12 @@ func (schema *Schema) validate() error {
}

func Load(path string) (*Schema, error) {
b, err := os.ReadFile(path)
dir, file := filepath.Split(path)
return LoadFS(os.DirFS(dir), file)
}

func LoadFS(fsys fs.FS, path string) (*Schema, error) {
b, err := fs.ReadFile(fsys, path)
if err != nil {
return nil, err
}
Expand Down
7 changes: 7 additions & 0 deletions libs/jsonschema/schema_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package jsonschema

import (
"os"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -305,3 +306,9 @@ func TestValidateSchemaSkippedPropertiesHaveDefaults(t *testing.T) {
err = s.validate()
assert.NoError(t, err)
}

func TestSchema_LoadFS(t *testing.T) {
fsys := os.DirFS("./testdata/schema-load-int")
_, err := LoadFS(fsys, "schema-valid.json")
assert.NoError(t, err)
}
47 changes: 47 additions & 0 deletions libs/template/builtin.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package template

import (
"embed"
"io/fs"
)

//go:embed all:templates
var builtinTemplates embed.FS

// BuiltinTemplate represents a template that is built into the CLI.
type BuiltinTemplate struct {
Name string
FS fs.FS
}

// Builtin returns the list of all built-in templates.
func Builtin() ([]BuiltinTemplate, error) {
templates, err := fs.Sub(builtinTemplates, "templates")
if err != nil {
return nil, err
}

entries, err := fs.ReadDir(templates, ".")
if err != nil {
return nil, err
}

var out []BuiltinTemplate
for _, entry := range entries {
if !entry.IsDir() {
continue
}

templateFS, err := fs.Sub(templates, entry.Name())
if err != nil {
return nil, err
}

out = append(out, BuiltinTemplate{
Name: entry.Name(),
FS: templateFS,
})
}

return out, nil
}
28 changes: 28 additions & 0 deletions libs/template/builtin_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package template

import (
"io/fs"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestBuiltin(t *testing.T) {
out, err := Builtin()
require.NoError(t, err)
assert.Len(t, out, 3)

// Confirm names.
assert.Equal(t, "dbt-sql", out[0].Name)
assert.Equal(t, "default-python", out[1].Name)
assert.Equal(t, "default-sql", out[2].Name)

// Confirm that the filesystems work.
_, err = fs.Stat(out[0].FS, `template/{{.project_name}}/dbt_project.yml.tmpl`)
assert.NoError(t, err)
_, err = fs.Stat(out[1].FS, `template/{{.project_name}}/tests/main_test.py.tmpl`)
assert.NoError(t, err)
_, err = fs.Stat(out[2].FS, `template/{{.project_name}}/src/orders_daily.sql.tmpl`)
assert.NoError(t, err)
}
6 changes: 3 additions & 3 deletions libs/template/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"io/fs"

"github.com/databricks/cli/libs/cmdio"
"github.com/databricks/cli/libs/jsonschema"
Expand All @@ -28,9 +29,8 @@ type config struct {
schema *jsonschema.Schema
}

func newConfig(ctx context.Context, schemaPath string) (*config, error) {
// Read config schema
schema, err := jsonschema.Load(schemaPath)
func newConfig(ctx context.Context, templateFS fs.FS, schemaPath string) (*config, error) {
schema, err := jsonschema.LoadFS(templateFS, schemaPath)
if err != nil {
return nil, err
}
Expand Down
29 changes: 16 additions & 13 deletions libs/template/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package template
import (
"context"
"fmt"
"os"
"path"
"path/filepath"
"testing"
"text/template"
Expand All @@ -16,7 +18,7 @@ func TestTemplateConfigAssignValuesFromFile(t *testing.T) {
testDir := "./testdata/config-assign-from-file"

ctx := context.Background()
c, err := newConfig(ctx, filepath.Join(testDir, "schema.json"))
c, err := newConfig(ctx, os.DirFS(testDir), "schema.json")
require.NoError(t, err)

err = c.assignValuesFromFile(filepath.Join(testDir, "config.json"))
Expand All @@ -32,7 +34,7 @@ func TestTemplateConfigAssignValuesFromFileDoesNotOverwriteExistingConfigs(t *te
testDir := "./testdata/config-assign-from-file"

ctx := context.Background()
c, err := newConfig(ctx, filepath.Join(testDir, "schema.json"))
c, err := newConfig(ctx, os.DirFS(testDir), "schema.json")
require.NoError(t, err)

c.values = map[string]any{
Expand All @@ -52,7 +54,7 @@ func TestTemplateConfigAssignValuesFromFileForInvalidIntegerValue(t *testing.T)
testDir := "./testdata/config-assign-from-file-invalid-int"

ctx := context.Background()
c, err := newConfig(ctx, filepath.Join(testDir, "schema.json"))
c, err := newConfig(ctx, os.DirFS(testDir), "schema.json")
require.NoError(t, err)

err = c.assignValuesFromFile(filepath.Join(testDir, "config.json"))
Expand All @@ -63,7 +65,7 @@ func TestTemplateConfigAssignValuesFromFileFiltersPropertiesNotInTheSchema(t *te
testDir := "./testdata/config-assign-from-file-unknown-property"

ctx := context.Background()
c, err := newConfig(ctx, filepath.Join(testDir, "schema.json"))
c, err := newConfig(ctx, os.DirFS(testDir), "schema.json")
require.NoError(t, err)

err = c.assignValuesFromFile(filepath.Join(testDir, "config.json"))
Expand All @@ -78,10 +80,10 @@ func TestTemplateConfigAssignValuesFromDefaultValues(t *testing.T) {
testDir := "./testdata/config-assign-from-default-value"

ctx := context.Background()
c, err := newConfig(ctx, filepath.Join(testDir, "schema.json"))
c, err := newConfig(ctx, os.DirFS(testDir), "schema.json")
require.NoError(t, err)

r, err := newRenderer(ctx, nil, nil, "./testdata/empty/template", "./testdata/empty/library", t.TempDir())
r, err := newRenderer(ctx, nil, nil, os.DirFS("."), "./testdata/empty/template", "./testdata/empty/library", t.TempDir())
require.NoError(t, err)

err = c.assignDefaultValues(r)
Expand All @@ -97,10 +99,10 @@ func TestTemplateConfigAssignValuesFromTemplatedDefaultValues(t *testing.T) {
testDir := "./testdata/config-assign-from-templated-default-value"

ctx := context.Background()
c, err := newConfig(ctx, filepath.Join(testDir, "schema.json"))
c, err := newConfig(ctx, os.DirFS(testDir), "schema.json")
require.NoError(t, err)

r, err := newRenderer(ctx, nil, nil, filepath.Join(testDir, "template/template"), filepath.Join(testDir, "template/library"), t.TempDir())
r, err := newRenderer(ctx, nil, nil, os.DirFS("."), path.Join(testDir, "template/template"), path.Join(testDir, "template/library"), t.TempDir())
require.NoError(t, err)

// Note: only the string value is templated.
Expand All @@ -116,7 +118,7 @@ func TestTemplateConfigAssignValuesFromTemplatedDefaultValues(t *testing.T) {

func TestTemplateConfigValidateValuesDefined(t *testing.T) {
ctx := context.Background()
c, err := newConfig(ctx, "testdata/config-test-schema/test-schema.json")
c, err := newConfig(ctx, os.DirFS("testdata/config-test-schema"), "test-schema.json")
require.NoError(t, err)

c.values = map[string]any{
Expand All @@ -131,7 +133,7 @@ func TestTemplateConfigValidateValuesDefined(t *testing.T) {

func TestTemplateConfigValidateTypeForValidConfig(t *testing.T) {
ctx := context.Background()
c, err := newConfig(ctx, "testdata/config-test-schema/test-schema.json")
c, err := newConfig(ctx, os.DirFS("testdata/config-test-schema"), "test-schema.json")
require.NoError(t, err)

c.values = map[string]any{
Expand All @@ -147,7 +149,7 @@ func TestTemplateConfigValidateTypeForValidConfig(t *testing.T) {

func TestTemplateConfigValidateTypeForUnknownField(t *testing.T) {
ctx := context.Background()
c, err := newConfig(ctx, "testdata/config-test-schema/test-schema.json")
c, err := newConfig(ctx, os.DirFS("testdata/config-test-schema"), "test-schema.json")
require.NoError(t, err)

c.values = map[string]any{
Expand All @@ -164,7 +166,7 @@ func TestTemplateConfigValidateTypeForUnknownField(t *testing.T) {

func TestTemplateConfigValidateTypeForInvalidType(t *testing.T) {
ctx := context.Background()
c, err := newConfig(ctx, "testdata/config-test-schema/test-schema.json")
c, err := newConfig(ctx, os.DirFS("testdata/config-test-schema"), "test-schema.json")
require.NoError(t, err)

c.values = map[string]any{
Expand Down Expand Up @@ -271,7 +273,8 @@ func TestTemplateEnumValidation(t *testing.T) {
}

func TestTemplateSchemaErrorsWithEmptyDescription(t *testing.T) {
_, err := newConfig(context.Background(), "./testdata/config-test-schema/invalid-test-schema.json")
ctx := context.Background()
_, err := newConfig(ctx, os.DirFS("./testdata/config-test-schema"), "invalid-test-schema.json")
assert.EqualError(t, err, "template property property-without-description is missing a description")
}

Expand Down
21 changes: 16 additions & 5 deletions libs/template/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ import (
"io/fs"
"os"
"path/filepath"

"github.com/databricks/cli/libs/filer"
"slices"
)

// Interface representing a file to be materialized from a template into a project
Expand All @@ -19,6 +18,10 @@ type file interface {

// Write file to disk at the destination path.
PersistToDisk() error

// contents returns the file contents as a byte slice.
// This is used for testing purposes.
contents() ([]byte, error)
}

type destinationPath struct {
Expand Down Expand Up @@ -46,8 +49,8 @@ type copyFile struct {

dstPath *destinationPath

// Filer rooted at template root. Used to read srcPath.
srcFiler filer.Filer
// [fs.FS] rooted at template root. Used to read srcPath.
srcFS fs.FS

// Relative path from template root for file to be copied.
srcPath string
Expand All @@ -63,7 +66,7 @@ func (f *copyFile) PersistToDisk() error {
if err != nil {
return err
}
srcFile, err := f.srcFiler.Read(f.ctx, f.srcPath)
srcFile, err := f.srcFS.Open(f.srcPath)
if err != nil {
return err
}
Expand All @@ -77,6 +80,10 @@ func (f *copyFile) PersistToDisk() error {
return err
}

func (f *copyFile) contents() ([]byte, error) {
return fs.ReadFile(f.srcFS, f.srcPath)
}

type inMemoryFile struct {
dstPath *destinationPath

Expand All @@ -99,3 +106,7 @@ func (f *inMemoryFile) PersistToDisk() error {
}
return os.WriteFile(path, f.content, f.perm)
}

func (f *inMemoryFile) contents() ([]byte, error) {
return slices.Clone(f.content), nil
}
Loading

0 comments on commit 4fea021

Please sign in to comment.