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

Add bundle init command and support for prompting user for input values #631

Merged
merged 22 commits into from
Aug 7, 2023

Conversation

shreyas-goenka
Copy link
Contributor

@shreyas-goenka shreyas-goenka commented Aug 2, 2023

Changes

This PR adds two features:

  1. The bundle init command
  2. Support for prompting for input values

In order to do this, this PR also introduces a new config struct which handles reading config files, prompting users and all validation steps before we materialize the template

With this PR users can start authoring custom templates, based on go text templates, for their projects / orgs.

Tests

Unit tests, both existing and new

@shreyas-goenka shreyas-goenka changed the title Add bundle init command and support for prompting user for input values [DRAFT] Add bundle init command and support for prompting user for input values Aug 2, 2023
@shreyas-goenka
Copy link
Contributor Author

How the prompting logic looks like. Here 1.1 is the default value for A:

Screen.Recording.2023-08-02.at.4.42.17.PM.mov

@shreyas-goenka
Copy link
Contributor Author

shreyas-goenka commented Aug 3, 2023

End to end flow works, with downloading the repo from github. tested with this repo: https://github.com/databricks/bundle-template-samples

@shreyas-goenka shreyas-goenka changed the title [DRAFT] Add bundle init command and support for prompting user for input values Add bundle init command and support for prompting user for input values Aug 3, 2023
cmd/bundle/init.go Outdated Show resolved Hide resolved
cmd/bundle/init.go Outdated Show resolved Hide resolved
cmd/bundle/init.go Outdated Show resolved Hide resolved
cmd/bundle/init.go Outdated Show resolved Hide resolved
libs/template/config.go Show resolved Hide resolved
libs/template/utils.go Outdated Show resolved Hide resolved
libs/template/utils_test.go Show resolved Hide resolved
libs/template/utils_test.go Show resolved Hide resolved
libs/template/validators_test.go Outdated Show resolved Hide resolved
cmd/bundle/init.go Show resolved Hide resolved
cmd/bundle/init_test.go Show resolved Hide resolved
@shreyas-goenka shreyas-goenka added this pull request to the merge queue Aug 7, 2023
Merged via the queue into main with commit 81ee031 Aug 7, 2023
4 checks passed
@shreyas-goenka shreyas-goenka deleted the add-prompt-and-init-cmd branch August 7, 2023 13:23
@pietern pietern mentioned this pull request Aug 10, 2023
pietern added a commit that referenced this pull request Aug 10, 2023
CLI:
 * Infer host from profile during `auth login` ([#629](#629)).

Bundles:
 * Extend deployment mode support ([#577](#577)).
 * Add validation for Git settings in bundles ([#578](#578)).
 * Only treat files with .tmpl extension as templates ([#594](#594)).
 * Add JSON schema validation for input template parameters ([#598](#598)).
 * Add DATABRICKS_BUNDLE_INCLUDE_PATHS to specify include paths through env vars ([#591](#591)).
 * Initialise a empty default bundle if BUNDLE_ROOT and DATABRICKS_BUNDLE_INCLUDES env vars are present ([#604](#604)).
 * Regenerate bundle resource structs from latest Terraform provider ([#633](#633)).
 * Fixed processing jobs libraries with remote path ([#638](#638)).
 * Add unit test for file name execution during rendering ([#640](#640)).
 * Add bundle init command and support for prompting user for input values ([#631](#631)).
 * Fix bundle git branch validation ([#645](#645)).

Internal:
 * Fix mkdir integration test on GCP ([#620](#620)).
 * Fix git clone integration test for non-existing repo ([#610](#610)).
 * Remove push to main trigger for build workflow ([#621](#621)).
 * Remove workflow to publish binaries to S3 ([#622](#622)).
 * Fix failing fs mkdir test on azure ([#627](#627)).
 * Print y/n options when displaying prompts using cmdio.Ask ([#650](#650)).

API Changes:
 * Changed `databricks account metastore-assignments create` command to not return anything.
 * Added `databricks account network-policy` command group.

OpenAPI commit 7b57ba3a53f4de3d049b6a24391fe5474212daf8 (2023-07-28)

Dependency updates:
 * Bump OpenAPI specification & Go SDK Version ([#624](#624)).
 * Bump golang.org/x/term from 0.10.0 to 0.11.0 ([#643](#643)).
 * Bump golang.org/x/text from 0.11.0 to 0.12.0 ([#642](#642)).
 * Bump golang.org/x/oauth2 from 0.10.0 to 0.11.0 ([#641](#641)).
github-merge-queue bot pushed a commit that referenced this pull request Aug 10, 2023
CLI:
* Infer host from profile during `auth login`
([#629](#629)).

Bundles:
* Extend deployment mode support
([#577](#577)).
* Add validation for Git settings in bundles
([#578](#578)).
* Only treat files with .tmpl extension as templates
([#594](#594)).
* Add JSON schema validation for input template parameters
([#598](#598)).
* Add DATABRICKS_BUNDLE_INCLUDE_PATHS to specify include paths through
env vars ([#591](#591)).
* Initialise a empty default bundle if BUNDLE_ROOT and
DATABRICKS_BUNDLE_INCLUDES env vars are present
([#604](#604)).
* Regenerate bundle resource structs from latest Terraform provider
([#633](#633)).
* Fixed processing jobs libraries with remote path
([#638](#638)).
* Add unit test for file name execution during rendering
([#640](#640)).
* Add bundle init command and support for prompting user for input
values ([#631](#631)).
* Fix bundle git branch validation
([#645](#645)).

Internal:
* Fix mkdir integration test on GCP
([#620](#620)).
* Fix git clone integration test for non-existing repo
([#610](#610)).
* Remove push to main trigger for build workflow
([#621](#621)).
* Remove workflow to publish binaries to S3
([#622](#622)).
* Fix failing fs mkdir test on azure
([#627](#627)).
* Print y/n options when displaying prompts using cmdio.Ask
([#650](#650)).

API Changes:
* Changed `databricks account metastore-assignments create` command to
not return anything.
 * Added `databricks account network-policy` command group.

OpenAPI commit 7b57ba3a53f4de3d049b6a24391fe5474212daf8 (2023-07-28)

Dependency updates:
* Bump OpenAPI specification & Go SDK Version
([#624](#624)).
* Bump golang.org/x/term from 0.10.0 to 0.11.0
([#643](#643)).
* Bump golang.org/x/text from 0.11.0 to 0.12.0
([#642](#642)).
* Bump golang.org/x/oauth2 from 0.10.0 to 0.11.0
([#641](#641)).
var projectDir string
cmd.Flags().StringVar(&configFile, "config-file", "", "File containing input parameters for template initialization.")
cmd.Flags().StringVar(&projectDir, "project-dir", "", "The project will be initialized in this directory.")
cmd.MarkFlagRequired("project-dir")
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we would use the current working directory by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to start out with a minimal implementation. We can add the default as a followup. Agreed though that the current directory makes sense.

// Download the template in a temporary directory
tmpDir := os.TempDir()
templateURL := templatePath
templateDir := filepath.Join(tmpDir, repoName(templateURL))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this matter if it is a temporary directory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just an implementation detail. I figured temp dir would be the best place to download the repo, and then delete it once its served its purpose for now.

return fmt.Errorf("failed to cast value %v of property %s from file %s to an integer: %w", floatVal, name, path, err)
}
configFromFile[name] = v
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This transformation would need to be done for any structure for which we have a JSON schema right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe. If a custom json unmarshaller we implement treats integer valued numbers as an int when type is ambigious, then we would not need this. Ofcourse doing this would have other side effects.

}

// Write configs from the file to the input map, not overwriting any existing
// configurations.
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the rationale for not overwriting existing values? Why would we have existing values?

Copy link
Contributor Author

@shreyas-goenka shreyas-goenka Aug 10, 2023

Choose a reason for hiding this comment

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

In the future we will add a --parameters flag which has a higher priority than the config-file. Those values would the existing ones mentioned here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants