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

Use fs.FS interface to read template #1910

Merged
merged 7 commits into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func getFsForNativeTemplate(name string) (fs.FS, error) {
func getFsForBuiltInTemplate(name string) (fs.FS, error) {

This can be confusing because we also have the nativeTemplates slice above, which are really first-party templates. To keep this explicit, we should rename nativeTemplates -> firstPartyTemplates as well (along with the associated methods).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. This will require a bit more work than just the rename, though, so I will address this in a follow-up PR.

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
Loading