-
Notifications
You must be signed in to change notification settings - Fork 154
Add bundle init command and support for prompting user for input values #631
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
Changes from all commits
ca4a68a
52c749e
418ab6c
3173c1f
c3d4684
c8734a5
0824d04
1d6156a
e03cc74
ab80b65
bbdb4df
e1a7f62
49a0062
03553f7
4e934cb
94c10c8
d6779d5
67827df
59647d0
db149ee
eed1566
b101ec2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| package bundle | ||
|
|
||
| import ( | ||
| "os" | ||
| "path/filepath" | ||
| "strings" | ||
|
|
||
| "github.com/databricks/cli/libs/git" | ||
| "github.com/databricks/cli/libs/template" | ||
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| var gitUrlPrefixes = []string{ | ||
| "https://", | ||
| "git@", | ||
| } | ||
|
|
||
| func isRepoUrl(url string) bool { | ||
| result := false | ||
| for _, prefix := range gitUrlPrefixes { | ||
| if strings.HasPrefix(url, prefix) { | ||
| result = true | ||
| break | ||
| } | ||
| } | ||
| return result | ||
| } | ||
|
|
||
| // Computes the repo name from the repo URL. Treats the last non empty word | ||
| // when splitting at '/' as the repo name. For example: for url git@github.com:databricks/cli.git | ||
| // the name would be "cli.git" | ||
| func repoName(url string) string { | ||
| parts := strings.Split(strings.TrimRight(url, "/"), "/") | ||
| return parts[len(parts)-1] | ||
| } | ||
|
|
||
| func newInitCommand() *cobra.Command { | ||
| cmd := &cobra.Command{ | ||
| Use: "init TEMPLATE_PATH", | ||
| Short: "Initialize Template", | ||
| Args: cobra.ExactArgs(1), | ||
| } | ||
|
|
||
| var configFile string | ||
| 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") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought we would use the current working directory by default.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
|
|
||
| cmd.RunE = func(cmd *cobra.Command, args []string) error { | ||
| templatePath := args[0] | ||
| ctx := cmd.Context() | ||
|
|
||
| if !isRepoUrl(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, projectDir) | ||
| } | ||
|
|
||
| // Download the template in a temporary directory | ||
| tmpDir := os.TempDir() | ||
| templateURL := templatePath | ||
| templateDir := filepath.Join(tmpDir, repoName(templateURL)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does this matter if it is a temporary directory?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| err := os.MkdirAll(templateDir, 0755) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| // TODO: Add automated test that the downloaded git repo is cleaned up. | ||
| err = git.Clone(ctx, templateURL, "", templateDir) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| defer os.RemoveAll(templateDir) | ||
|
|
||
| return template.Materialize(ctx, configFile, templateDir, projectDir) | ||
| } | ||
|
|
||
| return cmd | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| package bundle | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| func TestBundleInitIsRepoUrl(t *testing.T) { | ||
shreyas-goenka marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| assert.True(t, isRepoUrl("git@github.com:databricks/cli.git")) | ||
| assert.True(t, isRepoUrl("https://github.com/databricks/cli.git")) | ||
|
|
||
| assert.False(t, isRepoUrl("./local")) | ||
| assert.False(t, isRepoUrl("foo")) | ||
| } | ||
|
|
||
| func TestBundleInitRepoName(t *testing.T) { | ||
| // Test valid URLs | ||
| assert.Equal(t, "cli.git", repoName("git@github.com:databricks/cli.git")) | ||
| assert.Equal(t, "cli", repoName("https://github.com/databricks/cli/")) | ||
|
|
||
| // test invalid URLs. In these cases the error would be floated when the | ||
| // git clone operation fails. | ||
| assert.Equal(t, "git@github.com:databricks", repoName("git@github.com:databricks")) | ||
| assert.Equal(t, "invalid-url", repoName("invalid-url")) | ||
| assert.Equal(t, "www.github.com", repoName("https://www.github.com")) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,198 @@ | ||
| package template | ||
|
|
||
| import ( | ||
| "context" | ||
| "encoding/json" | ||
| "fmt" | ||
| "os" | ||
|
|
||
| "github.com/databricks/cli/libs/cmdio" | ||
| "github.com/databricks/cli/libs/jsonschema" | ||
| ) | ||
|
|
||
| type config struct { | ||
| ctx context.Context | ||
| values map[string]any | ||
| schema *jsonschema.Schema | ||
| } | ||
|
|
||
| func newConfig(ctx context.Context, schemaPath string) (*config, error) { | ||
| // Read config schema | ||
| schemaBytes, err := os.ReadFile(schemaPath) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| schema := &jsonschema.Schema{} | ||
| err = json.Unmarshal(schemaBytes, schema) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| // Return config | ||
| return &config{ | ||
| ctx: ctx, | ||
| schema: schema, | ||
| values: make(map[string]any, 0), | ||
| }, nil | ||
| } | ||
|
|
||
| // Reads json file at path and assigns values from the file | ||
| func (c *config) assignValuesFromFile(path string) error { | ||
| // Read the config file | ||
| configFromFile := make(map[string]any, 0) | ||
| b, err := os.ReadFile(path) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| err = json.Unmarshal(b, &configFromFile) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // Cast any integer properties, from float to integer. Required because | ||
| // the json unmarshaller treats all json numbers as floating point | ||
| for name, floatVal := range configFromFile { | ||
| property, ok := c.schema.Properties[name] | ||
| if !ok { | ||
| return fmt.Errorf("%s is not defined as an input parameter for the template", name) | ||
| } | ||
| if property.Type != jsonschema.IntegerType { | ||
| continue | ||
| } | ||
| v, err := toInteger(floatVal) | ||
| if err != nil { | ||
| 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 | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
|
|
||
| // Write configs from the file to the input map, not overwriting any existing | ||
| // configurations. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the future we will add a |
||
| for name, val := range configFromFile { | ||
| if _, ok := c.values[name]; ok { | ||
| continue | ||
| } | ||
| c.values[name] = val | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // Assigns default values from schema to input config map | ||
| func (c *config) assignDefaultValues() error { | ||
| for name, property := range c.schema.Properties { | ||
| // Config already has a value assigned | ||
| if _, ok := c.values[name]; ok { | ||
| continue | ||
| } | ||
|
|
||
| // No default value defined for the property | ||
| if property.Default == nil { | ||
| continue | ||
| } | ||
|
|
||
| // Assign default value if property is not an integer | ||
| if property.Type != jsonschema.IntegerType { | ||
| c.values[name] = property.Default | ||
| continue | ||
| } | ||
|
|
||
| // Cast default value to int before assigning to an integer configuration. | ||
| // Required because untyped field Default will read all numbers as floats | ||
| // during unmarshalling | ||
| v, err := toInteger(property.Default) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to cast default value %v of property %s to an integer: %w", property.Default, name, err) | ||
| } | ||
| c.values[name] = v | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // Prompts user for values for properties that do not have a value set yet | ||
| func (c *config) promptForValues() error { | ||
| for name, property := range c.schema.Properties { | ||
| // Config already has a value assigned | ||
| if _, ok := c.values[name]; ok { | ||
| continue | ||
| } | ||
|
|
||
| // Initialize Prompt dialog | ||
| var err error | ||
| prompt := cmdio.Prompt(c.ctx) | ||
| prompt.Label = property.Description | ||
| prompt.AllowEdit = true | ||
|
|
||
| // Compute default value to display by converting it to a string | ||
| if property.Default != nil { | ||
shreyas-goenka marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| prompt.Default, err = toString(property.Default, property.Type) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| // Get user input by running the prompt | ||
| userInput, err := prompt.Run() | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // Convert user input string back to a value | ||
| c.values[name], err = fromString(userInput, property.Type) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // Prompt user for any missing config values. Assign default values if | ||
| // terminal is not TTY | ||
| func (c *config) promptOrAssignDefaultValues() error { | ||
| if cmdio.IsOutTTY(c.ctx) && cmdio.IsInTTY(c.ctx) { | ||
| return c.promptForValues() | ||
| } | ||
| return c.assignDefaultValues() | ||
| } | ||
|
|
||
| // Validates the configuration. If passes, the configuration is ready to be used | ||
| // to initialize the template. | ||
| func (c *config) validate() error { | ||
| validateFns := []func() error{ | ||
| c.validateValuesDefined, | ||
| c.validateValuesType, | ||
| } | ||
|
|
||
| for _, fn := range validateFns { | ||
| err := fn() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // Validates all input properties have a user defined value assigned to them | ||
| func (c *config) validateValuesDefined() error { | ||
| for k := range c.schema.Properties { | ||
| if _, ok := c.values[k]; ok { | ||
| continue | ||
| } | ||
| return fmt.Errorf("no value has been assigned to input parameter %s", k) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // Validates the types of all input properties values match their types defined in the schema | ||
| func (c *config) validateValuesType() error { | ||
| for k, v := range c.values { | ||
| fieldInfo, ok := c.schema.Properties[k] | ||
| if !ok { | ||
| return fmt.Errorf("%s is not defined as an input parameter for the template", k) | ||
| } | ||
| err := validateType(v, fieldInfo.Type) | ||
| if err != nil { | ||
| return fmt.Errorf("incorrect type for %s. %w", k, err) | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.