From 592e1111b77ca9d4e2ecdfe63e40a8dbeef544d8 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 20 Nov 2024 13:53:25 +0100 Subject: [PATCH] Update filenames used by bundle generate to use `..yml` (#1901) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Changes Update filenames used by bundle generate to use '.resource-type.yml' Similar to [Add sub-extension to resource files in built-in templates by shreyas-goenka · Pull Request #1777 · databricks/cli](https://github.com/databricks/cli/pull/1777) --------- Co-authored-by: shreyas-goenka <88374338+shreyas-goenka@users.noreply.github.com> --- cmd/bundle/generate/generate_test.go | 122 +++++++++++++++++++++++++- cmd/bundle/generate/job.go | 14 ++- cmd/bundle/generate/pipeline.go | 14 ++- internal/bundle/bind_resource_test.go | 2 +- 4 files changed, 147 insertions(+), 5 deletions(-) diff --git a/cmd/bundle/generate/generate_test.go b/cmd/bundle/generate/generate_test.go index 943f721c..bc1549e6 100644 --- a/cmd/bundle/generate/generate_test.go +++ b/cmd/bundle/generate/generate_test.go @@ -3,8 +3,10 @@ package generate import ( "bytes" "context" + "errors" "fmt" "io" + "io/fs" "os" "path/filepath" "testing" @@ -90,7 +92,7 @@ func TestGeneratePipelineCommand(t *testing.T) { err := cmd.RunE(cmd, []string{}) require.NoError(t, err) - data, err := os.ReadFile(filepath.Join(configDir, "test_pipeline.yml")) + data, err := os.ReadFile(filepath.Join(configDir, "test_pipeline.pipeline.yml")) require.NoError(t, err) require.Equal(t, fmt.Sprintf(`resources: pipelines: @@ -186,7 +188,123 @@ func TestGenerateJobCommand(t *testing.T) { err := cmd.RunE(cmd, []string{}) require.NoError(t, err) - data, err := os.ReadFile(filepath.Join(configDir, "test_job.yml")) + data, err := os.ReadFile(filepath.Join(configDir, "test_job.job.yml")) + require.NoError(t, err) + + require.Equal(t, fmt.Sprintf(`resources: + jobs: + test_job: + name: test-job + job_clusters: + - new_cluster: + custom_tags: + "Tag1": "24X7-1234" + - new_cluster: + spark_conf: + "spark.databricks.delta.preview.enabled": "true" + tasks: + - task_key: notebook_task + notebook_task: + notebook_path: %s + parameters: + - name: empty + default: "" +`, filepath.Join("..", "src", "notebook.py")), string(data)) + + data, err = os.ReadFile(filepath.Join(srcDir, "notebook.py")) + require.NoError(t, err) + require.Equal(t, "# Databricks notebook source\nNotebook content", string(data)) +} + +func touchEmptyFile(t *testing.T, path string) { + err := os.MkdirAll(filepath.Dir(path), 0700) + require.NoError(t, err) + f, err := os.Create(path) + require.NoError(t, err) + f.Close() +} + +func TestGenerateJobCommandOldFileRename(t *testing.T) { + cmd := NewGenerateJobCommand() + + root := t.TempDir() + b := &bundle.Bundle{ + BundleRootPath: root, + } + + m := mocks.NewMockWorkspaceClient(t) + b.SetWorkpaceClient(m.WorkspaceClient) + + jobsApi := m.GetMockJobsAPI() + jobsApi.EXPECT().Get(mock.Anything, jobs.GetJobRequest{JobId: 1234}).Return(&jobs.Job{ + Settings: &jobs.JobSettings{ + Name: "test-job", + JobClusters: []jobs.JobCluster{ + {NewCluster: compute.ClusterSpec{ + CustomTags: map[string]string{ + "Tag1": "24X7-1234", + }, + }}, + {NewCluster: compute.ClusterSpec{ + SparkConf: map[string]string{ + "spark.databricks.delta.preview.enabled": "true", + }, + }}, + }, + Tasks: []jobs.Task{ + { + TaskKey: "notebook_task", + NotebookTask: &jobs.NotebookTask{ + NotebookPath: "/test/notebook", + }, + }, + }, + Parameters: []jobs.JobParameterDefinition{ + { + Name: "empty", + Default: "", + }, + }, + }, + }, nil) + + workspaceApi := m.GetMockWorkspaceAPI() + workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/test/notebook").Return(&workspace.ObjectInfo{ + ObjectType: workspace.ObjectTypeNotebook, + Language: workspace.LanguagePython, + Path: "/test/notebook", + }, nil) + + notebookContent := io.NopCloser(bytes.NewBufferString("# Databricks notebook source\nNotebook content")) + workspaceApi.EXPECT().Download(mock.Anything, "/test/notebook", mock.Anything).Return(notebookContent, nil) + + cmd.SetContext(bundle.Context(context.Background(), b)) + cmd.Flag("existing-job-id").Value.Set("1234") + + configDir := filepath.Join(root, "resources") + cmd.Flag("config-dir").Value.Set(configDir) + + srcDir := filepath.Join(root, "src") + cmd.Flag("source-dir").Value.Set(srcDir) + + var key string + cmd.Flags().StringVar(&key, "key", "test_job", "") + + // Create an old generated file first + oldFilename := filepath.Join(configDir, "test_job.yml") + touchEmptyFile(t, oldFilename) + + // Having an existing files require --force flag to regenerate them + cmd.Flag("force").Value.Set("true") + + err := cmd.RunE(cmd, []string{}) + require.NoError(t, err) + + // Make sure file do not exists after the run + _, err = os.Stat(oldFilename) + require.True(t, errors.Is(err, fs.ErrNotExist)) + + data, err := os.ReadFile(filepath.Join(configDir, "test_job.job.yml")) require.NoError(t, err) require.Equal(t, fmt.Sprintf(`resources: diff --git a/cmd/bundle/generate/job.go b/cmd/bundle/generate/job.go index 99bc6166..9ac41e3c 100644 --- a/cmd/bundle/generate/job.go +++ b/cmd/bundle/generate/job.go @@ -1,7 +1,9 @@ package generate import ( + "errors" "fmt" + "io/fs" "os" "path/filepath" @@ -83,7 +85,17 @@ func NewGenerateJobCommand() *cobra.Command { return err } - filename := filepath.Join(configDir, fmt.Sprintf("%s.yml", jobKey)) + oldFilename := filepath.Join(configDir, fmt.Sprintf("%s.yml", jobKey)) + filename := filepath.Join(configDir, fmt.Sprintf("%s.job.yml", jobKey)) + + // User might continuously run generate command to update their bundle jobs with any changes made in Databricks UI. + // Due to changing in the generated file names, we need to first rename existing resource file to the new name. + // Otherwise users can end up with duplicated resources. + err = os.Rename(oldFilename, filename) + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("failed to rename file %s. DABs uses the resource type as a sub-extension for generated content, please rename it to %s, err: %w", oldFilename, filename, err) + } + saver := yamlsaver.NewSaverWithStyle(map[string]yaml.Style{ // Including all JobSettings and nested fields which are map[string]string type "spark_conf": yaml.DoubleQuotedStyle, diff --git a/cmd/bundle/generate/pipeline.go b/cmd/bundle/generate/pipeline.go index bd973fe0..910baa45 100644 --- a/cmd/bundle/generate/pipeline.go +++ b/cmd/bundle/generate/pipeline.go @@ -1,7 +1,9 @@ package generate import ( + "errors" "fmt" + "io/fs" "os" "path/filepath" @@ -83,7 +85,17 @@ func NewGeneratePipelineCommand() *cobra.Command { return err } - filename := filepath.Join(configDir, fmt.Sprintf("%s.yml", pipelineKey)) + oldFilename := filepath.Join(configDir, fmt.Sprintf("%s.yml", pipelineKey)) + filename := filepath.Join(configDir, fmt.Sprintf("%s.pipeline.yml", pipelineKey)) + + // User might continuously run generate command to update their bundle jobs with any changes made in Databricks UI. + // Due to changing in the generated file names, we need to first rename existing resource file to the new name. + // Otherwise users can end up with duplicated resources. + err = os.Rename(oldFilename, filename) + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("failed to rename file %s. DABs uses the resource type as a sub-extension for generated content, please rename it to %s, err: %w", oldFilename, filename, err) + } + saver := yamlsaver.NewSaverWithStyle( // Including all PipelineSpec and nested fields which are map[string]string type map[string]yaml.Style{ diff --git a/internal/bundle/bind_resource_test.go b/internal/bundle/bind_resource_test.go index 2449c31f..8cc5da53 100644 --- a/internal/bundle/bind_resource_test.go +++ b/internal/bundle/bind_resource_test.go @@ -166,7 +166,7 @@ func TestAccGenerateAndBind(t *testing.T) { _, err = os.Stat(filepath.Join(bundleRoot, "src", "test.py")) require.NoError(t, err) - matches, err := filepath.Glob(filepath.Join(bundleRoot, "resources", "test_job_key.yml")) + matches, err := filepath.Glob(filepath.Join(bundleRoot, "resources", "test_job_key.job.yml")) require.NoError(t, err) require.Len(t, matches, 1)