Skip to content

Commit

Permalink
Added ErrNotWorkspaceClient (#596)
Browse files Browse the repository at this point in the history
## Changes

Analogous to #238.

This enables the CLI to return an error if a user attempts to use
configuration for an account client with a workspace client.

Also see databricks/cli#721.

## Tests

New and existing integration tests pass (tested at workspace and account
level).
  • Loading branch information
pietern authored Aug 31, 2023
1 parent 9ba9a68 commit 51b4f2b
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 7 deletions.
11 changes: 10 additions & 1 deletion .codegen/workspaces.go.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ type WorkspaceClient struct {
{{end}}{{end}}
}

// NewWorkspaceClient creates new Databricks SDK client for Workspaces or
var ErrNotWorkspaceClient = errors.New("invalid Databricks Workspace configuration")

// NewWorkspaceClient creates new Databricks SDK client for Workspaces or
// returns error in case configuration is wrong
func NewWorkspaceClient(c ...*Config) (*WorkspaceClient, error) {
var cfg *config.Config
Expand All @@ -28,6 +30,13 @@ func NewWorkspaceClient(c ...*Config) (*WorkspaceClient, error) {
// default config
cfg = &config.Config{}
}
err := cfg.EnsureResolved()
if err != nil {
return nil, err
}
if cfg.IsAccountClient() {
return nil, ErrNotWorkspaceClient
}
apiClient, err := client.New(cfg)
if err != nil {
return nil, err
Expand Down
32 changes: 26 additions & 6 deletions internal/acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package internal

import (
"context"
"os"
"os/exec"
"testing"

Expand All @@ -15,10 +16,10 @@ import (

func TestAccDefaultCredentials(t *testing.T) {
t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV"))
w := databricks.Must(databricks.NewWorkspaceClient())
if w.Config.IsAccountClient() {
t.SkipNow()
if os.Getenv("DATABRICKS_ACCOUNT_ID") != "" {
skipf(t)("Skipping workspace test on account level")
}
w := databricks.Must(databricks.NewWorkspaceClient())
ctx := context.Background()
versions, err := w.Clusters.SparkVersions(ctx)
require.NoError(t, err)
Expand All @@ -34,12 +35,13 @@ func TestAccDefaultCredentials(t *testing.T) {
// TODO: add CredentialProviderChain

func TestAccExplicitDatabricksCfg(t *testing.T) {
t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV"))
if os.Getenv("DATABRICKS_ACCOUNT_ID") != "" {
skipf(t)("Skipping workspace test on account level")
}
w := databricks.Must(databricks.NewWorkspaceClient(&databricks.Config{
Profile: GetEnvOrSkipTest(t, "DATABRICKS_CONFIG_PROFILE"),
}))
if w.Config.IsAccountClient() {
t.SkipNow()
}
ctx := context.Background()
versions, err := w.Clusters.SparkVersions(ctx)
require.NoError(t, err)
Expand Down Expand Up @@ -106,3 +108,21 @@ func TestAccExplicitAzureSpnAuth(t *testing.T) {
assert.NoError(t, err)
assert.NotEmpty(t, v)
}

func TestAccErrNotAccountClient(t *testing.T) {
workspaceTest(t)

// Confirm that we get an error when trying to create an account client
// if the configuration indicates a workspace.
_, err := databricks.NewAccountClient()
assert.ErrorIs(t, err, databricks.ErrNotAccountClient)
}

func TestAccErrNotWorkspaceClient(t *testing.T) {
accountTest(t)

// Confirm that we get an error when trying to create a workspace client
// if the configuration indicates an account.
_, err := databricks.NewWorkspaceClient()
assert.ErrorIs(t, err, databricks.ErrNotWorkspaceClient)
}
11 changes: 11 additions & 0 deletions workspace_client.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 51b4f2b

Please sign in to comment.