From 84a869868f1aedf8347bfd7e4aa8911f61b51028 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 21 Oct 2024 21:29:58 +0200 Subject: [PATCH 1/8] Add command to open the web UI for a resource --- bundle/resources/completion.go | 39 +++++++++ bundle/resources/completion_test.go | 1 + bundle/resources/keys.go | 64 ++++++++++++++ bundle/resources/keys_test.go | 1 + cmd/bundle/bundle.go | 1 + cmd/bundle/open.go | 125 ++++++++++++++++++++++++++++ 6 files changed, 231 insertions(+) create mode 100644 bundle/resources/completion.go create mode 100644 bundle/resources/completion_test.go create mode 100644 bundle/resources/keys.go create mode 100644 bundle/resources/keys_test.go create mode 100644 cmd/bundle/open.go diff --git a/bundle/resources/completion.go b/bundle/resources/completion.go new file mode 100644 index 0000000000..6e16b2b4c1 --- /dev/null +++ b/bundle/resources/completion.go @@ -0,0 +1,39 @@ +package resources + +import ( + "github.com/databricks/cli/bundle" + "golang.org/x/exp/maps" +) + +func CompletionMap(b *bundle.Bundle) map[string]string { + out := make(map[string]string) + keyOnly, keyWithType := Keys(b) + + // Keep track of resources we have seen by their fully qualified key. + seen := make(map[string]bool) + + // First add resources that can be identified by key alone. + for k, v := range keyOnly { + // Invariant: len(v) >= 1. See [ResourceKeys]. + if len(v) == 1 { + seen[v[0].key] = true + out[k] = v[0].resource.GetName() + } + } + + // Then add resources that can only be identified by their type and key. + for k, v := range keyWithType { + // Invariant: len(v) == 1. See [ResourceKeys]. + _, ok := seen[v[0].key] + if ok { + continue + } + out[k] = v[0].resource.GetName() + } + + return out +} + +func Completions(b *bundle.Bundle) []string { + return maps.Keys(CompletionMap(b)) +} diff --git a/bundle/resources/completion_test.go b/bundle/resources/completion_test.go new file mode 100644 index 0000000000..18d6395a74 --- /dev/null +++ b/bundle/resources/completion_test.go @@ -0,0 +1 @@ +package resources diff --git a/bundle/resources/keys.go b/bundle/resources/keys.go new file mode 100644 index 0000000000..90a5dbe773 --- /dev/null +++ b/bundle/resources/keys.go @@ -0,0 +1,64 @@ +package resources + +import ( + "fmt" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" +) + +type pair struct { + key string + resource config.ConfigResource +} + +// lookup maps identifiers to a list of resources that match that identifier. +// The list can have more than 1 entry if resources of different types use the +// same key. When this happens, the user should disambiguate between them. +type lookup map[string][]pair + +// Keys computes maps that index resources by their key (e.g. `my_job`) and by their key +// prefixed by their type (e.g. `jobs.my_job`). The resource key alone may be ambiguous (it is +// possible for resources of different types to have the same key), but the key prefixed by +// the type is guaranteed to be unique. +func Keys(b *bundle.Bundle) (keyOnly lookup, keyWithType lookup) { + keyOnly = make(lookup) + keyWithType = make(lookup) + + // Collect all resources by their key and prefixed key. + for _, group := range b.Config.Resources.AllResources() { + typ := group.Description.PluralName + for k, v := range group.Resources { + kt := fmt.Sprintf("%s.%s", typ, k) + p := pair{key: kt, resource: v} + keyOnly[k] = append(keyOnly[k], p) + keyWithType[kt] = append(keyWithType[kt], p) + } + } + + return +} + +// Lookup returns the resource with the given key. +// It first attempts to find a resource with the key alone. +// If this fails, it tries the key prefixed by the resource type. +// If this also fails, it returns an error. +func Lookup(b *bundle.Bundle, key string) (config.ConfigResource, error) { + keyOnly, keyWithType := Keys(b) + + // First try to find the resource by key alone. + if res, ok := keyOnly[key]; ok { + if len(res) == 1 { + return res[0].resource, nil + } + } + + // Then try to find the resource by key and type. + if res, ok := keyWithType[key]; ok { + if len(res) == 1 { + return res[0].resource, nil + } + } + + return nil, fmt.Errorf("resource with key %q not found", key) +} diff --git a/bundle/resources/keys_test.go b/bundle/resources/keys_test.go new file mode 100644 index 0000000000..18d6395a74 --- /dev/null +++ b/bundle/resources/keys_test.go @@ -0,0 +1 @@ +package resources diff --git a/cmd/bundle/bundle.go b/cmd/bundle/bundle.go index 0880c9c442..fb88cd7d05 100644 --- a/cmd/bundle/bundle.go +++ b/cmd/bundle/bundle.go @@ -27,5 +27,6 @@ func New() *cobra.Command { cmd.AddCommand(newGenerateCommand()) cmd.AddCommand(newDebugCommand()) cmd.AddCommand(deployment.NewDeploymentCommand()) + cmd.AddCommand(newOpenCommand()) return cmd } diff --git a/cmd/bundle/open.go b/cmd/bundle/open.go new file mode 100644 index 0000000000..c4d7e7e384 --- /dev/null +++ b/cmd/bundle/open.go @@ -0,0 +1,125 @@ +package bundle + +import ( + "errors" + "fmt" + "os" + "path/filepath" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/bundle/deploy/terraform" + "github.com/databricks/cli/bundle/phases" + "github.com/databricks/cli/bundle/resources" + "github.com/databricks/cli/cmd/bundle/utils" + "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/cmdio" + "github.com/spf13/cobra" + + "github.com/pkg/browser" +) + +func newOpenCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "open", + Short: "Open the web UI for a resource", + Args: root.MaximumNArgs(1), + } + + var forcePull bool + cmd.Flags().BoolVar(&forcePull, "force-pull", false, "Skip local cache and load the state from the remote workspace") + + cmd.RunE = func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + b, diags := utils.ConfigureBundleWithVariables(cmd) + if err := diags.Error(); err != nil { + return diags.Error() + } + + diags = bundle.Apply(ctx, b, phases.Initialize()) + if err := diags.Error(); err != nil { + return err + } + + // If no arguments are specified, prompt the user to select something to run. + if len(args) == 0 && cmdio.IsPromptSupported(ctx) { + // Invert completions from KEY -> NAME, to NAME -> KEY. + inv := make(map[string]string) + for k, v := range resources.CompletionMap(b) { + inv[v] = k + } + id, err := cmdio.Select(ctx, inv, "Resource to open") + if err != nil { + return err + } + args = append(args, id) + } + + if len(args) < 1 { + return fmt.Errorf("expected a KEY of the resource to open") + } + + cacheDir, err := terraform.Dir(ctx, b) + if err != nil { + return err + } + _, stateFileErr := os.Stat(filepath.Join(cacheDir, terraform.TerraformStateFileName)) + _, configFileErr := os.Stat(filepath.Join(cacheDir, terraform.TerraformConfigFileName)) + noCache := errors.Is(stateFileErr, os.ErrNotExist) || errors.Is(configFileErr, os.ErrNotExist) + + if forcePull || noCache { + diags = bundle.Apply(ctx, b, bundle.Seq( + terraform.StatePull(), + terraform.Interpolate(), + terraform.Write(), + )) + if err := diags.Error(); err != nil { + return err + } + } + + diags = bundle.Apply(ctx, b, bundle.Seq( + terraform.Load(), + mutator.InitializeURLs(), + )) + if err := diags.Error(); err != nil { + return err + } + + // Locate resource to open. + resource, err := resources.Lookup(b, args[0]) + if err != nil { + return err + } + + // Confirm that the resource has a URL. + url := resource.GetURL() + if url == "" { + return errors.New("resource does not have a URL associated with it (has it been deployed?)") + } + + return browser.OpenURL(url) + } + + cmd.ValidArgsFunction = func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + b, diags := root.MustConfigureBundle(cmd) + if err := diags.Error(); err != nil { + cobra.CompErrorln(err.Error()) + return nil, cobra.ShellCompDirectiveError + } + + // No completion in the context of a bundle. + // Source and destination paths are taken from bundle configuration. + if b == nil { + return nil, cobra.ShellCompDirectiveNoFileComp + } + + if len(args) == 0 { + return resources.Completions(b), cobra.ShellCompDirectiveNoFileComp + } else { + return nil, cobra.ShellCompDirectiveNoFileComp + } + } + + return cmd +} From 15dc49f4aa551a5cac9d7d2dd19490e8d5660286 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 23 Oct 2024 15:57:38 +0200 Subject: [PATCH 2/8] Update how resource lookup is done to account for duplicates --- bundle/resources/completion.go | 39 +++---------- bundle/resources/completion_test.go | 36 ++++++++++++ bundle/resources/keys.go | 64 ---------------------- bundle/resources/keys_test.go | 1 - bundle/resources/lookup.go | 59 ++++++++++++++++++++ bundle/resources/lookup_test.go | 85 +++++++++++++++++++++++++++++ cmd/bundle/open.go | 57 ++++++++++++------- 7 files changed, 226 insertions(+), 115 deletions(-) delete mode 100644 bundle/resources/keys.go delete mode 100644 bundle/resources/keys_test.go create mode 100644 bundle/resources/lookup.go create mode 100644 bundle/resources/lookup_test.go diff --git a/bundle/resources/completion.go b/bundle/resources/completion.go index 6e16b2b4c1..2e033813b6 100644 --- a/bundle/resources/completion.go +++ b/bundle/resources/completion.go @@ -1,39 +1,16 @@ package resources -import ( - "github.com/databricks/cli/bundle" - "golang.org/x/exp/maps" -) +import "github.com/databricks/cli/bundle" -func CompletionMap(b *bundle.Bundle) map[string]string { - out := make(map[string]string) - keyOnly, keyWithType := Keys(b) - - // Keep track of resources we have seen by their fully qualified key. - seen := make(map[string]bool) - - // First add resources that can be identified by key alone. - for k, v := range keyOnly { - // Invariant: len(v) >= 1. See [ResourceKeys]. - if len(v) == 1 { - seen[v[0].key] = true - out[k] = v[0].resource.GetName() - } - } - - // Then add resources that can only be identified by their type and key. - for k, v := range keyWithType { - // Invariant: len(v) == 1. See [ResourceKeys]. - _, ok := seen[v[0].key] - if ok { +// Completions returns the same as [References] except +// that every key maps directly to a single reference. +func Completions(b *bundle.Bundle) map[string]Reference { + out := make(map[string]Reference) + for k, refs := range References(b) { + if len(refs) != 1 { continue } - out[k] = v[0].resource.GetName() + out[k] = refs[0] } - return out } - -func Completions(b *bundle.Bundle) []string { - return maps.Keys(CompletionMap(b)) -} diff --git a/bundle/resources/completion_test.go b/bundle/resources/completion_test.go index 18d6395a74..10c0c0ba67 100644 --- a/bundle/resources/completion_test.go +++ b/bundle/resources/completion_test.go @@ -1 +1,37 @@ package resources + +import ( + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/databricks-sdk-go/service/jobs" + "github.com/stretchr/testify/assert" +) + +func TestCompletions(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "foo": {}, + "bar": { + JobSettings: &jobs.JobSettings{ + Name: "Bar job", + }, + }, + }, + Pipelines: map[string]*resources.Pipeline{ + "foo": {}, + }, + }, + }, + } + + // Test that this skips duplicates and only includes unambiguous completions. + out := Completions(b) + if assert.Len(t, out, 1) { + assert.Contains(t, out, "bar") + } +} diff --git a/bundle/resources/keys.go b/bundle/resources/keys.go deleted file mode 100644 index 90a5dbe773..0000000000 --- a/bundle/resources/keys.go +++ /dev/null @@ -1,64 +0,0 @@ -package resources - -import ( - "fmt" - - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config" -) - -type pair struct { - key string - resource config.ConfigResource -} - -// lookup maps identifiers to a list of resources that match that identifier. -// The list can have more than 1 entry if resources of different types use the -// same key. When this happens, the user should disambiguate between them. -type lookup map[string][]pair - -// Keys computes maps that index resources by their key (e.g. `my_job`) and by their key -// prefixed by their type (e.g. `jobs.my_job`). The resource key alone may be ambiguous (it is -// possible for resources of different types to have the same key), but the key prefixed by -// the type is guaranteed to be unique. -func Keys(b *bundle.Bundle) (keyOnly lookup, keyWithType lookup) { - keyOnly = make(lookup) - keyWithType = make(lookup) - - // Collect all resources by their key and prefixed key. - for _, group := range b.Config.Resources.AllResources() { - typ := group.Description.PluralName - for k, v := range group.Resources { - kt := fmt.Sprintf("%s.%s", typ, k) - p := pair{key: kt, resource: v} - keyOnly[k] = append(keyOnly[k], p) - keyWithType[kt] = append(keyWithType[kt], p) - } - } - - return -} - -// Lookup returns the resource with the given key. -// It first attempts to find a resource with the key alone. -// If this fails, it tries the key prefixed by the resource type. -// If this also fails, it returns an error. -func Lookup(b *bundle.Bundle, key string) (config.ConfigResource, error) { - keyOnly, keyWithType := Keys(b) - - // First try to find the resource by key alone. - if res, ok := keyOnly[key]; ok { - if len(res) == 1 { - return res[0].resource, nil - } - } - - // Then try to find the resource by key and type. - if res, ok := keyWithType[key]; ok { - if len(res) == 1 { - return res[0].resource, nil - } - } - - return nil, fmt.Errorf("resource with key %q not found", key) -} diff --git a/bundle/resources/keys_test.go b/bundle/resources/keys_test.go deleted file mode 100644 index 18d6395a74..0000000000 --- a/bundle/resources/keys_test.go +++ /dev/null @@ -1 +0,0 @@ -package resources diff --git a/bundle/resources/lookup.go b/bundle/resources/lookup.go new file mode 100644 index 0000000000..5cf8b724e5 --- /dev/null +++ b/bundle/resources/lookup.go @@ -0,0 +1,59 @@ +package resources + +import ( + "fmt" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" +) + +// Reference is a reference to a resource. +// It includes the resource key, the resource type description, and a reference to the resource itself. +type Reference struct { + Key string + Description config.ResourceDescription + Resource config.ConfigResource +} + +// Map is the core type for resource lookup and completion. +type Map map[string][]Reference + +// References returns a map of resource keys to a slice of [Reference]. +// While its return type allows for multiple resources to share the same key, +// this is confirmed not to happen in the [validate.UniqueResourceKeys] mutator. +func References(b *bundle.Bundle) Map { + output := make(Map) + + // Collect map of resource references indexed by their keys. + for _, group := range b.Config.Resources.AllResources() { + for k, v := range group.Resources { + output[k] = append(output[k], Reference{ + Key: k, + Description: group.Description, + Resource: v, + }) + } + } + + return output +} + +// Lookup returns the resource with the specified key. +// If the key maps to more than one resource, an error is returned. +// If the key does not map to any resource, an error is returned. +func Lookup(b *bundle.Bundle, key string) (config.ConfigResource, error) { + refs := References(b) + res, ok := refs[key] + if !ok { + return nil, fmt.Errorf("resource with key %q not found", key) + } + + switch { + case len(res) == 1: + return res[0].Resource, nil + case len(res) > 1: + return nil, fmt.Errorf("multiple resources with key %q found", key) + default: + panic("unreachable") + } +} diff --git a/bundle/resources/lookup_test.go b/bundle/resources/lookup_test.go new file mode 100644 index 0000000000..20bc5189bd --- /dev/null +++ b/bundle/resources/lookup_test.go @@ -0,0 +1,85 @@ +package resources + +import ( + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/databricks-sdk-go/service/jobs" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLookup_EmptyBundle(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{}, + }, + } + + out, err := Lookup(b, "foo") + require.Error(t, err) + assert.Nil(t, out) + assert.ErrorContains(t, err, "resource with key \"foo\" not found") +} + +func TestLookup_NotFound(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "foo": {}, + "bar": {}, + }, + }, + }, + } + + out, err := Lookup(b, "qux") + require.Error(t, err) + assert.Nil(t, out) + assert.ErrorContains(t, err, `resource with key "qux" not found`) +} + +func TestLookup_MultipleFound(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "foo": {}, + }, + Pipelines: map[string]*resources.Pipeline{ + "foo": {}, + }, + }, + }, + } + + out, err := Lookup(b, "foo") + require.Error(t, err) + assert.Nil(t, out) + assert.ErrorContains(t, err, `multiple resources with key "foo" found`) +} + +func TestLookup_Nominal(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "foo": { + JobSettings: &jobs.JobSettings{ + Name: "Foo job", + }, + }, + }, + }, + }, + } + + out, err := Lookup(b, "foo") + require.NoError(t, err) + if assert.NotNil(t, out) { + assert.Equal(t, "Foo job", out.GetName()) + } +} diff --git a/cmd/bundle/open.go b/cmd/bundle/open.go index c4d7e7e384..cf3df7a57d 100644 --- a/cmd/bundle/open.go +++ b/cmd/bundle/open.go @@ -1,6 +1,7 @@ package bundle import ( + "context" "errors" "fmt" "os" @@ -15,10 +16,40 @@ import ( "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/libs/cmdio" "github.com/spf13/cobra" + "golang.org/x/exp/maps" "github.com/pkg/browser" ) +func promptOpenArgument(ctx context.Context, b *bundle.Bundle) (string, error) { + // Compute map of "Human readable name of resource" -> "resource key". + inv := make(map[string]string) + for k, ref := range resources.Completions(b) { + title := fmt.Sprintf("%s: %s", ref.Description.SingularTitle, ref.Resource.GetName()) + inv[title] = k + } + + key, err := cmdio.Select(ctx, inv, "Resource to open") + if err != nil { + return "", err + } + + return key, nil +} + +func resolveOpenArgument(ctx context.Context, b *bundle.Bundle, args []string) (string, error) { + // If no arguments are specified, prompt the user to select something to run. + if len(args) == 0 && cmdio.IsPromptSupported(ctx) { + return promptOpenArgument(ctx, b) + } + + if len(args) < 1 { + return "", fmt.Errorf("expected a KEY of the resource to open") + } + + return args[0], nil +} + func newOpenCommand() *cobra.Command { cmd := &cobra.Command{ Use: "open", @@ -41,22 +72,9 @@ func newOpenCommand() *cobra.Command { return err } - // If no arguments are specified, prompt the user to select something to run. - if len(args) == 0 && cmdio.IsPromptSupported(ctx) { - // Invert completions from KEY -> NAME, to NAME -> KEY. - inv := make(map[string]string) - for k, v := range resources.CompletionMap(b) { - inv[v] = k - } - id, err := cmdio.Select(ctx, inv, "Resource to open") - if err != nil { - return err - } - args = append(args, id) - } - - if len(args) < 1 { - return fmt.Errorf("expected a KEY of the resource to open") + arg, err := resolveOpenArgument(ctx, b, args) + if err != nil { + return err } cacheDir, err := terraform.Dir(ctx, b) @@ -87,7 +105,7 @@ func newOpenCommand() *cobra.Command { } // Locate resource to open. - resource, err := resources.Lookup(b, args[0]) + resource, err := resources.Lookup(b, arg) if err != nil { return err } @@ -95,7 +113,7 @@ func newOpenCommand() *cobra.Command { // Confirm that the resource has a URL. url := resource.GetURL() if url == "" { - return errors.New("resource does not have a URL associated with it (has it been deployed?)") + return fmt.Errorf("resource does not have a URL associated with it (has it been deployed?)") } return browser.OpenURL(url) @@ -115,7 +133,8 @@ func newOpenCommand() *cobra.Command { } if len(args) == 0 { - return resources.Completions(b), cobra.ShellCompDirectiveNoFileComp + completions := resources.Completions(b) + return maps.Keys(completions), cobra.ShellCompDirectiveNoFileComp } else { return nil, cobra.ShellCompDirectiveNoFileComp } From 57cf0d634b44d010ac6991744627c18cbce15341 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 23 Oct 2024 16:00:10 +0200 Subject: [PATCH 3/8] Short help --- cmd/bundle/open.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/bundle/open.go b/cmd/bundle/open.go index cf3df7a57d..2eb70f20f6 100644 --- a/cmd/bundle/open.go +++ b/cmd/bundle/open.go @@ -53,7 +53,7 @@ func resolveOpenArgument(ctx context.Context, b *bundle.Bundle, args []string) ( func newOpenCommand() *cobra.Command { cmd := &cobra.Command{ Use: "open", - Short: "Open the web UI for a resource", + Short: "Open a resource in the browser", Args: root.MaximumNArgs(1), } From e6a8d8a4caae643863fa0bf5e510b651eaadfb07 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 23 Oct 2024 16:57:46 +0200 Subject: [PATCH 4/8] Add lookup by fully qualified name --- bundle/resources/completion.go | 3 ++- bundle/resources/lookup.go | 42 ++++++++++++++++++++------------- bundle/resources/lookup_test.go | 8 +++++++ 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/bundle/resources/completion.go b/bundle/resources/completion.go index 2e033813b6..3ce0510a9c 100644 --- a/bundle/resources/completion.go +++ b/bundle/resources/completion.go @@ -6,7 +6,8 @@ import "github.com/databricks/cli/bundle" // that every key maps directly to a single reference. func Completions(b *bundle.Bundle) map[string]Reference { out := make(map[string]Reference) - for k, refs := range References(b) { + keyOnlyRefs, _ := References(b) + for k, refs := range keyOnlyRefs { if len(refs) != 1 { continue } diff --git a/bundle/resources/lookup.go b/bundle/resources/lookup.go index 5cf8b724e5..930356f72c 100644 --- a/bundle/resources/lookup.go +++ b/bundle/resources/lookup.go @@ -8,9 +8,8 @@ import ( ) // Reference is a reference to a resource. -// It includes the resource key, the resource type description, and a reference to the resource itself. +// It includes the resource type description, and a reference to the resource itself. type Reference struct { - Key string Description config.ResourceDescription Resource config.ConfigResource } @@ -18,40 +17,51 @@ type Reference struct { // Map is the core type for resource lookup and completion. type Map map[string][]Reference -// References returns a map of resource keys to a slice of [Reference]. -// While its return type allows for multiple resources to share the same key, +// References returns maps of resource keys to a slice of [Reference]. +// +// The first map is indexed by the resource key only. +// The second map is indexed by the resource type name and its key. +// +// While the return types allows for multiple resources to share the same key, // this is confirmed not to happen in the [validate.UniqueResourceKeys] mutator. -func References(b *bundle.Bundle) Map { - output := make(Map) +func References(b *bundle.Bundle) (Map, Map) { + keyOnly := make(Map) + keyWithType := make(Map) // Collect map of resource references indexed by their keys. for _, group := range b.Config.Resources.AllResources() { for k, v := range group.Resources { - output[k] = append(output[k], Reference{ - Key: k, + ref := Reference{ Description: group.Description, Resource: v, - }) + } + + kt := fmt.Sprintf("%s.%s", group.Description.PluralName, k) + keyOnly[k] = append(keyOnly[k], ref) + keyWithType[kt] = append(keyWithType[kt], ref) } } - return output + return keyOnly, keyWithType } // Lookup returns the resource with the specified key. // If the key maps to more than one resource, an error is returned. // If the key does not map to any resource, an error is returned. func Lookup(b *bundle.Bundle, key string) (config.ConfigResource, error) { - refs := References(b) - res, ok := refs[key] + keyOnlyRefs, keyWithTypeRefs := References(b) + refs, ok := keyOnlyRefs[key] if !ok { - return nil, fmt.Errorf("resource with key %q not found", key) + refs, ok = keyWithTypeRefs[key] + if !ok { + return nil, fmt.Errorf("resource with key %q not found", key) + } } switch { - case len(res) == 1: - return res[0].Resource, nil - case len(res) > 1: + case len(refs) == 1: + return refs[0].Resource, nil + case len(refs) > 1: return nil, fmt.Errorf("multiple resources with key %q found", key) default: panic("unreachable") diff --git a/bundle/resources/lookup_test.go b/bundle/resources/lookup_test.go index 20bc5189bd..2d38e71bb9 100644 --- a/bundle/resources/lookup_test.go +++ b/bundle/resources/lookup_test.go @@ -77,9 +77,17 @@ func TestLookup_Nominal(t *testing.T) { }, } + // Lookup by key only. out, err := Lookup(b, "foo") require.NoError(t, err) if assert.NotNil(t, out) { assert.Equal(t, "Foo job", out.GetName()) } + + // Lookup by type and key. + out, err = Lookup(b, "jobs.foo") + require.NoError(t, err) + if assert.NotNil(t, out) { + assert.Equal(t, "Foo job", out.GetName()) + } } From 9a0f169840691f4dbc9d8cc820f012b5cc94dcb9 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 24 Oct 2024 11:10:50 +0200 Subject: [PATCH 5/8] Rename test --- bundle/resources/completion_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/resources/completion_test.go b/bundle/resources/completion_test.go index 10c0c0ba67..ddb17119dc 100644 --- a/bundle/resources/completion_test.go +++ b/bundle/resources/completion_test.go @@ -10,7 +10,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestCompletions(t *testing.T) { +func TestCompletions_SkipDuplicates(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ Resources: config.Resources{ From 94d3b5e0da664d157e5068ea71286e2529fa95ac Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 24 Oct 2024 11:11:42 +0200 Subject: [PATCH 6/8] Simplify test --- bundle/resources/completion_test.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/bundle/resources/completion_test.go b/bundle/resources/completion_test.go index ddb17119dc..36ad1a06a7 100644 --- a/bundle/resources/completion_test.go +++ b/bundle/resources/completion_test.go @@ -6,7 +6,6 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/stretchr/testify/assert" ) @@ -16,11 +15,7 @@ func TestCompletions_SkipDuplicates(t *testing.T) { Resources: config.Resources{ Jobs: map[string]*resources.Job{ "foo": {}, - "bar": { - JobSettings: &jobs.JobSettings{ - Name: "Bar job", - }, - }, + "bar": {}, }, Pipelines: map[string]*resources.Pipeline{ "foo": {}, From 1c5540201c19faa15ec4a0412dc3e5e2b9aad117 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 24 Oct 2024 11:28:02 +0200 Subject: [PATCH 7/8] Make Lookup return Reference object --- bundle/resources/lookup.go | 8 ++++---- bundle/resources/lookup_test.go | 13 +++++-------- cmd/bundle/open.go | 6 +++--- 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/bundle/resources/lookup.go b/bundle/resources/lookup.go index 930356f72c..74aec531ac 100644 --- a/bundle/resources/lookup.go +++ b/bundle/resources/lookup.go @@ -48,21 +48,21 @@ func References(b *bundle.Bundle) (Map, Map) { // Lookup returns the resource with the specified key. // If the key maps to more than one resource, an error is returned. // If the key does not map to any resource, an error is returned. -func Lookup(b *bundle.Bundle, key string) (config.ConfigResource, error) { +func Lookup(b *bundle.Bundle, key string) (Reference, error) { keyOnlyRefs, keyWithTypeRefs := References(b) refs, ok := keyOnlyRefs[key] if !ok { refs, ok = keyWithTypeRefs[key] if !ok { - return nil, fmt.Errorf("resource with key %q not found", key) + return Reference{}, fmt.Errorf("resource with key %q not found", key) } } switch { case len(refs) == 1: - return refs[0].Resource, nil + return refs[0], nil case len(refs) > 1: - return nil, fmt.Errorf("multiple resources with key %q found", key) + return Reference{}, fmt.Errorf("multiple resources with key %q found", key) default: panic("unreachable") } diff --git a/bundle/resources/lookup_test.go b/bundle/resources/lookup_test.go index 2d38e71bb9..5bcb615701 100644 --- a/bundle/resources/lookup_test.go +++ b/bundle/resources/lookup_test.go @@ -18,9 +18,8 @@ func TestLookup_EmptyBundle(t *testing.T) { }, } - out, err := Lookup(b, "foo") + _, err := Lookup(b, "foo") require.Error(t, err) - assert.Nil(t, out) assert.ErrorContains(t, err, "resource with key \"foo\" not found") } @@ -36,9 +35,8 @@ func TestLookup_NotFound(t *testing.T) { }, } - out, err := Lookup(b, "qux") + _, err := Lookup(b, "qux") require.Error(t, err) - assert.Nil(t, out) assert.ErrorContains(t, err, `resource with key "qux" not found`) } @@ -56,9 +54,8 @@ func TestLookup_MultipleFound(t *testing.T) { }, } - out, err := Lookup(b, "foo") + _, err := Lookup(b, "foo") require.Error(t, err) - assert.Nil(t, out) assert.ErrorContains(t, err, `multiple resources with key "foo" found`) } @@ -81,13 +78,13 @@ func TestLookup_Nominal(t *testing.T) { out, err := Lookup(b, "foo") require.NoError(t, err) if assert.NotNil(t, out) { - assert.Equal(t, "Foo job", out.GetName()) + assert.Equal(t, "Foo job", out.Resource.GetName()) } // Lookup by type and key. out, err = Lookup(b, "jobs.foo") require.NoError(t, err) if assert.NotNil(t, out) { - assert.Equal(t, "Foo job", out.GetName()) + assert.Equal(t, "Foo job", out.Resource.GetName()) } } diff --git a/cmd/bundle/open.go b/cmd/bundle/open.go index 2eb70f20f6..a2ad32fd80 100644 --- a/cmd/bundle/open.go +++ b/cmd/bundle/open.go @@ -38,7 +38,7 @@ func promptOpenArgument(ctx context.Context, b *bundle.Bundle) (string, error) { } func resolveOpenArgument(ctx context.Context, b *bundle.Bundle, args []string) (string, error) { - // If no arguments are specified, prompt the user to select something to run. + // If no arguments are specified, prompt the user to select the resource to open. if len(args) == 0 && cmdio.IsPromptSupported(ctx) { return promptOpenArgument(ctx, b) } @@ -105,13 +105,13 @@ func newOpenCommand() *cobra.Command { } // Locate resource to open. - resource, err := resources.Lookup(b, arg) + ref, err := resources.Lookup(b, arg) if err != nil { return err } // Confirm that the resource has a URL. - url := resource.GetURL() + url := ref.Resource.GetURL() if url == "" { return fmt.Errorf("resource does not have a URL associated with it (has it been deployed?)") } From 03e9b5fa1a4ef106e1292f137c6fdde94188de9f Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 24 Oct 2024 11:33:15 +0200 Subject: [PATCH 8/8] Simplify test --- bundle/resources/lookup_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/bundle/resources/lookup_test.go b/bundle/resources/lookup_test.go index 5bcb615701..d2092c23da 100644 --- a/bundle/resources/lookup_test.go +++ b/bundle/resources/lookup_test.go @@ -76,15 +76,13 @@ func TestLookup_Nominal(t *testing.T) { // Lookup by key only. out, err := Lookup(b, "foo") - require.NoError(t, err) - if assert.NotNil(t, out) { + if assert.NoError(t, err) { assert.Equal(t, "Foo job", out.Resource.GetName()) } // Lookup by type and key. out, err = Lookup(b, "jobs.foo") - require.NoError(t, err) - if assert.NotNil(t, out) { + if assert.NoError(t, err) { assert.Equal(t, "Foo job", out.Resource.GetName()) } }