Refactor `bundle init` #2074

shreyas-goenka · 2025-01-03T10:58:54Z

Summary of changes

This PR introduces three new abstractions:

Resolver: Resolves which reader and writer to use for a template.
Writer: Writes a template project to disk. Prompts the user if necessary.
Reader: Reads a template specification from disk, built into the CLI or from GitHub.

Introducing these abstractions helps decouple reading a template from writing it. When I tried adding telemetry for the bundle init command, I noticed that the code in cmd/init.go was getting convoluted and hard to test. A future change could have accidentally logged PII when a user initialised a custom template.

Hedging against that risk is important here because we use a generic untyped map<string, string> representation in the backend to log telemetry for the databricks bundle init. Otherwise, we risk accidentally breaking our compliance with our centralization requirements.

Details

After this PR there are two classes of templates that can be initialized:

A databricks template: This could be a builtin template or a template outside the CLI like mlops-stacks, which is still owned and managed by Databricks. These templates log their telemetry arguments and template name.
A custom template: These are templates created by and managed by the end user. In these templates we do not log the template name and args. Instead a generic placeholder string of "custom" is logged in our telemetry system.

NOTE: The functionality of the databricks bundle init command remains the same after this PR. Only the internal abstractions used are changed.

Tests

New unit tests. Existing golden and unit tests. Also a fair bit of manual testing.

github-actions · 2025-01-03T13:03:03Z

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/cli

Inputs:

PR number: 2074
Commit SHA: ed24f2880b101b8a671f681d9e87ea332aa33ebd

Checks will be approved automatically on success.

shreyas-goenka · 2025-01-06T11:25:45Z

cmd/bundle/init.go

-}
-
-// Return template descriptions for command-line help
-func nativeTemplateHelpDescriptions() string {


moved totemplate.HelpDescriptions()

shreyas-goenka · 2025-01-06T11:26:18Z

cmd/bundle/init.go

-}
-
-// Return template options for an interactive prompt
-func nativeTemplateOptions() []cmdio.Tuple {


moved to template.options()

shreyas-goenka · 2025-01-06T11:27:02Z

cmd/bundle/init.go

-	return names
-}
-
-func getNativeTemplateByDescription(description string) string {


Moved inline to template. SelectTemplate()

shreyas-goenka · 2025-01-06T11:27:39Z

cmd/bundle/init.go

-	return ""
-}
-
-func getUrlForNativeTemplate(name string) string {


Replaced by template.Get()

shreyas-goenka · 2025-01-06T11:27:57Z

cmd/bundle/init.go

-	return ""
-}
-
-func getFsForNativeTemplate(name string) (fs.FS, error) {


Replaced by template.Reader.FS() method.

shreyas-goenka · 2025-01-06T11:29:15Z

cmd/bundle/init.go

-	return parts[len(parts)-1]
-}
-
-func constructOutputFiler(ctx context.Context, outputDir string) (filer.Filer, error) {


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

shreyas-goenka · 2025-01-06T11:37:57Z

cmd/bundle/init.go

@@ -198,92 +38,32 @@ See https://docs.databricks.com/en/dev-tools/bundles/templates.html for more inf

 	cmd.PreRunE = root.MustWorkspaceClient
 	cmd.RunE = func(cmd *cobra.Command, args []string) error {
-		if tag != "" && branch != "" {


Moved over to template.Resolver.Resolve. This allows us to easily write unit tests for the function.

shreyas-goenka · 2025-01-06T12:35:26Z

libs/filer/workspace_files_extensions_client.go

@@ -16,7 +16,7 @@ import (
 	"github.com/databricks/databricks-sdk-go/service/workspace"
 )

-type workspaceFilesExtensionsClient struct {
+type WorkspaceFilesExtensionsClient struct {


Made public for assertion in TestDefaultWriterConfigureOnDBR

shreyas-goenka · 2025-01-06T12:37:07Z

libs/template/materialize.go

-//	configFilePath: file path containing user defined config values
-//	templateFS: 	root of the template definition
-//	outputFiler: 	filer to use for writing the initialized template
-func Materialize(ctx context.Context, configFilePath string, templateFS fs.FS, outputFiler filer.Filer) error {


logic Moved to defaultWriter{} struct in this library

shreyas-goenka · 2025-01-06T13:10:32Z

libs/template/writer.go

+type Writer interface {
+	Configure(ctx context.Context, configPath, outputDir string) error
+	Materialize(ctx context.Context, r Reader) error
+	LogTelemetry(ctx context.Context) error


This function does nothing right now. Telemetry functionality will be added in a followup PR.

shreyas-goenka · 2025-01-06T13:44:42Z

Integration tests were green, and after a while, I made a few linting commits. Triggered another run of the integration tests.

denik

It's not completely clear what parts are just moved and which are new, could you summarize that in PR description?

libs/template/reader.go

libs/template/reader_test.go

libs/template/resolver_test.go

shreyas-goenka · 2025-01-09T11:18:02Z

@denik Thanks! I redid the PR summary. Hopefully that makes it more legible what exactly changed.

Refactor bundle init

070ae2f

shreyas-goenka temporarily deployed to test-trigger-is January 3, 2025 10:58 — with GitHub Actions Inactive

shreyas-goenka temporarily deployed to test-trigger-is January 3, 2025 10:59 — with GitHub Actions Inactive

undo test changes

daa2b91

shreyas-goenka temporarily deployed to test-trigger-is January 3, 2025 11:00 — with GitHub Actions Inactive

remove final telemetry bits

a743139

shreyas-goenka temporarily deployed to test-trigger-is January 3, 2025 11:02 — with GitHub Actions Inactive

add resolver

d238dd8

shreyas-goenka temporarily deployed to test-trigger-is January 3, 2025 11:58 — with GitHub Actions Inactive

add unit tests for reader

2965c30

shreyas-goenka temporarily deployed to test-trigger-is January 3, 2025 13:00 — with GitHub Actions Inactive

more test

ed24f28

shreyas-goenka temporarily deployed to test-trigger-is January 3, 2025 13:02 — with GitHub Actions Inactive

shreyas-goenka temporarily deployed to test-trigger-is January 3, 2025 13:03 — with GitHub Actions Inactive

fix tests and lint

af89580

shreyas-goenka had a problem deploying to test-trigger-is January 6, 2025 08:11 — with GitHub Actions Failure

fix test

9f83fe0

shreyas-goenka had a problem deploying to test-trigger-is January 6, 2025 10:10 — with GitHub Actions Failure

fix another bug

00266cb

shreyas-goenka had a problem deploying to test-trigger-is January 6, 2025 10:28 — with GitHub Actions Failure

skip test on windows

2b1c5cb

shreyas-goenka had a problem deploying to test-trigger-is January 6, 2025 11:20 — with GitHub Actions Failure

add unit tests for resolver

8607e08

shreyas-goenka had a problem deploying to test-trigger-is January 6, 2025 12:23 — with GitHub Actions Failure

add test for the get method

39dfe05

shreyas-goenka had a problem deploying to test-trigger-is January 6, 2025 12:42 — with GitHub Actions Failure

custom rename to avoid break

4755db4

shreyas-goenka had a problem deploying to test-trigger-is January 6, 2025 12:57 — with GitHub Actions Failure

-

0a68fb3

shreyas-goenka had a problem deploying to test-trigger-is January 6, 2025 13:00 — with GitHub Actions Failure

more test

8202aaf

shreyas-goenka had a problem deploying to test-trigger-is January 6, 2025 13:08 — with GitHub Actions Failure

lint

4743095

shreyas-goenka had a problem deploying to test-trigger-is January 6, 2025 13:12 — with GitHub Actions Failure

cleanup a bit

57a7519

shreyas-goenka had a problem deploying to test-trigger-is January 6, 2025 13:19 — with GitHub Actions Failure

fix aliasing

38d47e6

shreyas-goenka had a problem deploying to test-trigger-is January 6, 2025 13:25 — with GitHub Actions Failure

fix aliasing

ad28ce1

shreyas-goenka had a problem deploying to test-trigger-is January 6, 2025 13:28 — with GitHub Actions Failure

shreyas-goenka commented Jan 6, 2025

View reviewed changes

shreyas-goenka marked this pull request as ready for review January 6, 2025 13:42

shreyas-goenka requested review from pietern, andrewnester and denik as code owners January 6, 2025 13:42

Merge remote-tracking branch 'origin' into refactor-bundle-init-squashed

f394606

shreyas-goenka had a problem deploying to test-trigger-is January 6, 2025 13:45 — with GitHub Actions Failure

denik reviewed Jan 7, 2025

View reviewed changes

libs/template/reader.go Outdated Show resolved Hide resolved

libs/template/reader_test.go Outdated Show resolved Hide resolved

libs/template/resolver_test.go Outdated Show resolved Hide resolved

shreyas-goenka added 2 commits January 9, 2025 16:30

remove fail reader

2a0dbde

address comments

a2a3ae7

shreyas-goenka requested a review from denik January 9, 2025 11:17

merge

30d683a

shreyas-goenka temporarily deployed to test-trigger-is January 9, 2025 11:19 — with GitHub Actions Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor `bundle init` #2074

Refactor `bundle init` #2074

shreyas-goenka commented Jan 3, 2025 •

edited

Loading

github-actions bot commented Jan 3, 2025

shreyas-goenka Jan 6, 2025

shreyas-goenka Jan 6, 2025

shreyas-goenka Jan 6, 2025

shreyas-goenka Jan 6, 2025

shreyas-goenka Jan 6, 2025

shreyas-goenka Jan 6, 2025

shreyas-goenka Jan 6, 2025

shreyas-goenka Jan 6, 2025

shreyas-goenka Jan 6, 2025

shreyas-goenka Jan 6, 2025

shreyas-goenka commented Jan 6, 2025

denik left a comment

shreyas-goenka commented Jan 9, 2025

Refactor bundle init #2074

Are you sure you want to change the base?

Refactor bundle init #2074

Conversation

shreyas-goenka commented Jan 3, 2025 • edited Loading

Summary of changes

Details

Tests

github-actions bot commented Jan 3, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shreyas-goenka commented Jan 6, 2025

denik left a comment

Choose a reason for hiding this comment

shreyas-goenka commented Jan 9, 2025

Refactor `bundle init` #2074

Refactor `bundle init` #2074

shreyas-goenka commented Jan 3, 2025 •

edited

Loading