From ae59d071992a17a17e4b60348899179e2614d969 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Fri, 15 Nov 2024 10:26:47 +0100 Subject: [PATCH] fixes --- cmd/bundle/generate/generate_test.go | 118 +++++++++++++++++++++++++++ cmd/bundle/generate/job.go | 3 + cmd/bundle/generate/pipeline.go | 5 +- 3 files changed, 125 insertions(+), 1 deletion(-) diff --git a/cmd/bundle/generate/generate_test.go b/cmd/bundle/generate/generate_test.go index bfc9611ad..bc1549e64 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" @@ -213,3 +215,119 @@ func TestGenerateJobCommand(t *testing.T) { 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: + 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)) +} diff --git a/cmd/bundle/generate/job.go b/cmd/bundle/generate/job.go index 1bf9dab7b..9ac41e3cb 100644 --- a/cmd/bundle/generate/job.go +++ b/cmd/bundle/generate/job.go @@ -88,6 +88,9 @@ func NewGenerateJobCommand() *cobra.Command { 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) diff --git a/cmd/bundle/generate/pipeline.go b/cmd/bundle/generate/pipeline.go index 9bce0b552..910baa45f 100644 --- a/cmd/bundle/generate/pipeline.go +++ b/cmd/bundle/generate/pipeline.go @@ -88,9 +88,12 @@ func NewGeneratePipelineCommand() *cobra.Command { 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 resource type as sub extension for generated content, please rename to %s, err: %w", oldFilename, filename, err) + 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(