Skip to content

Commit

Permalink
squash all commits
Browse files Browse the repository at this point in the history
  • Loading branch information
shreyas-goenka committed Jan 20, 2025
1 parent 41a21af commit 9012c4c
Show file tree
Hide file tree
Showing 19 changed files with 620 additions and 18 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ jobs:
make vendor
pip3 install wheel
- name: Run tests
run: make test
- name: Run tests with coverage
run: make cover

golangci:
needs: cleanups
Expand Down
15 changes: 7 additions & 8 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ default: vendor fmt lint
PACKAGES=./acceptance/... ./libs/... ./internal/... ./cmd/... ./bundle/... .

GOTESTSUM_FORMAT ?= pkgname-and-test-fails
GOTESTSUM_CMD ?= gotestsum --format ${GOTESTSUM_FORMAT} --no-summary=skipped


lint:
golangci-lint run --fix
Expand All @@ -18,22 +20,19 @@ fmt:
golangci-lint run --enable-only="gofmt,gofumpt,goimports" --fix ./...

test:
gotestsum --format ${GOTESTSUM_FORMAT} --no-summary=skipped -- ${PACKAGES}
${GOTESTSUM_CMD} -- ${PACKAGES}

cover:
gotestsum --format ${GOTESTSUM_FORMAT} --no-summary=skipped -- -coverprofile=coverage.txt ${PACKAGES}

showcover:
go tool cover -html=coverage.txt

acc-cover:
rm -fr ./acceptance/build/cover/
CLI_GOCOVERDIR=build/cover go test ./acceptance
CLI_GOCOVERDIR=build/cover ${GOTESTSUM_CMD} -- -coverprofile=coverage.txt ${PACKAGES}
rm -fr ./acceptance/build/cover-merged/
mkdir -p acceptance/build/cover-merged/
go tool covdata merge -i $$(printf '%s,' acceptance/build/cover/* | sed 's/,$$//') -o acceptance/build/cover-merged/
go tool covdata textfmt -i acceptance/build/cover-merged -o coverage-acceptance.txt

showcover:
go tool cover -html=coverage.txt

acc-showcover:
go tool cover -html=coverage-acceptance.txt

Expand Down
1 change: 0 additions & 1 deletion acceptance/help/output.txt
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ Marketplace

Apps
apps Apps run directly on a customer’s Databricks instance, integrate with their data, use and extend Databricks services, and enable users to interact through single sign-on.
apps Apps run directly on a customer’s Databricks instance, integrate with their data, use and extend Databricks services, and enable users to interact through single sign-on.

Clean Rooms
clean-room-assets Clean room assets are data and code objects — Tables, volumes, and notebooks that are shared with the clean room.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,17 +72,18 @@ func (m *applySourceLinkedDeploymentPreset) Apply(ctx context.Context, b *bundle
return diags
}

// This mutator runs before workspace paths are defaulted so it's safe to check for the user-defined value
if b.Config.Workspace.FilePath != "" && config.IsExplicitlyEnabled(b.Config.Presets.SourceLinkedDeployment) {
path := dyn.NewPath(dyn.Key("targets"), dyn.Key(target), dyn.Key("workspace"), dyn.Key("file_path"))

path := dyn.NewPath(dyn.Key("workspace"), dyn.Key("file_path"))
diags = diags.Append(
diag.Diagnostic{
Severity: diag.Warning,
Summary: "workspace.file_path setting will be ignored in source-linked deployment mode",
Detail: "In source-linked deployment files are not copied to the destination and resources use source files instead",
Paths: []dyn.Path{
path[2:],
path,
},
Locations: b.Config.GetLocations(path[2:].String()),
Locations: b.Config.GetLocations(path.String()),
},
)
}
Expand Down
1 change: 1 addition & 0 deletions bundle/deploy/metadata/compute.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ func (m *compute) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics {

// Set file upload destination of the bundle in metadata
b.Metadata.Config.Workspace.FilePath = b.Config.Workspace.FilePath
// In source-linked deployment files are not copied and resources use source files, therefore we use sync path as file path in metadata
if config.IsExplicitlyEnabled(b.Config.Presets.SourceLinkedDeployment) {
b.Metadata.Config.Workspace.FilePath = b.SyncRootPath
}
Expand Down
2 changes: 1 addition & 1 deletion bundle/internal/schema/annotations.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ github.com/databricks/cli/bundle/config.Bundle:
The name of the bundle.
"uuid":
"description": |-
PLACEHOLDER
Reserved. A Universally Unique Identifier (UUID) for the bundle that uniquely identifies the bundle in internal Databricks systems. This is generated when a bundle project is initialized using a Databricks template (using the `databricks bundle init` command).
github.com/databricks/cli/bundle/config.Deployment:
"fail_on_active_runs":
"description": |-
Expand Down
1 change: 1 addition & 0 deletions bundle/schema/jsonschema.json

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

2 changes: 0 additions & 2 deletions cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"github.com/databricks/cli/cmd/sync"
"github.com/databricks/cli/cmd/version"
"github.com/databricks/cli/cmd/workspace"
"github.com/databricks/cli/cmd/workspace/apps"
"github.com/spf13/cobra"
)

Expand Down Expand Up @@ -68,7 +67,6 @@ func New(ctx context.Context) *cobra.Command {

// Add other subcommands.
cli.AddCommand(api.New())
cli.AddCommand(apps.New())
cli.AddCommand(auth.New())
cli.AddCommand(bundle.New())
cli.AddCommand(configure.New())
Expand Down
91 changes: 91 additions & 0 deletions integration/libs/telemetry/telemetry_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package telemetry_test

import (
"context"
"net/http"
"reflect"
"testing"
"time"

"github.com/databricks/cli/integration/internal/acc"
"github.com/databricks/cli/libs/telemetry"
"github.com/databricks/cli/libs/telemetry/protos"
"github.com/databricks/databricks-sdk-go/client"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// Wrapper to capture the response from the API client since that's not directly
// accessible from the logger.
type apiClientWrapper struct {
response *telemetry.ResponseBody
apiClient *client.DatabricksClient
}

func (wrapper *apiClientWrapper) Do(ctx context.Context, method, path string,
headers map[string]string, request, response any,
visitors ...func(*http.Request) error,
) error {
err := wrapper.apiClient.Do(ctx, method, path, headers, request, response, visitors...)
wrapper.response = response.(*telemetry.ResponseBody)
return err
}

func TestTelemetryLogger(t *testing.T) {
events := []telemetry.DatabricksCliLog{
{
CliTestEvent: &protos.CliTestEvent{
Name: protos.DummyCliEnumValue1,
},
},
{
BundleInitEvent: &protos.BundleInitEvent{
Uuid: uuid.New().String(),
TemplateName: "abc",
TemplateEnumArgs: []protos.BundleInitTemplateEnumArg{
{
Key: "a",
Value: "b",
},
{
Key: "c",
Value: "d",
},
},
},
},
}

assert.Equal(t, len(events), reflect.TypeOf(telemetry.DatabricksCliLog{}).NumField(),
"Number of events should match the number of fields in DatabricksCliLog. Please add a new event to this test.")

ctx, w := acc.WorkspaceTest(t)
ctx = telemetry.WithDefaultLogger(ctx)

// Extend the maximum wait time for the telemetry flush just for this test.
oldV := telemetry.MaxAdditionalWaitTime
telemetry.MaxAdditionalWaitTime = 1 * time.Hour
t.Cleanup(func() {
telemetry.MaxAdditionalWaitTime = oldV
})

for _, event := range events {
telemetry.Log(ctx, event)
}

apiClient, err := client.New(w.W.Config)
require.NoError(t, err)

// Flush the events.
wrapper := &apiClientWrapper{
apiClient: apiClient,
}
telemetry.Flush(ctx, wrapper)

// Assert that the events were logged.
assert.Equal(t, telemetry.ResponseBody{
NumProtoSuccess: int64(len(events)),
Errors: []telemetry.LogError{},
}, *wrapper.response)
}
19 changes: 19 additions & 0 deletions libs/telemetry/api.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package telemetry

// RequestBody is the request body type bindings for the /telemetry-ext API endpoint.
type RequestBody struct {
UploadTime int64 `json:"uploadTime"`
Items []string `json:"items"`
ProtoLogs []string `json:"protoLogs"`
}

// ResponseBody is the response body type bindings for the /telemetry-ext API endpoint.
type ResponseBody struct {
Errors []LogError `json:"errors"`
NumProtoSuccess int64 `json:"numProtoSuccess"`
}

type LogError struct {
Message string `json:"message"`
ErrorType string `json:"ErrorType"`
}
62 changes: 62 additions & 0 deletions libs/telemetry/context.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package telemetry

import (
"context"
"fmt"
)

// Private type to store the telemetry logger in the context
type telemetryLogger int

// Key to store the telemetry logger in the context
var telemetryLoggerKey telemetryLogger

func WithDefaultLogger(ctx context.Context) context.Context {
v := ctx.Value(telemetryLoggerKey)

// If no logger is set in the context, set the default logger.
if v == nil {
nctx := context.WithValue(ctx, telemetryLoggerKey, &defaultLogger{})
return nctx
}

switch v.(type) {
case *defaultLogger:
panic(fmt.Errorf("default telemetry logger already set in the context: %T", v))
case *mockLogger:
// Do nothing. Unit and integration tests set the mock logger in the context
// to avoid making actual API calls. Thus WithDefaultLogger should silently
// ignore the mock logger.
default:
panic(fmt.Errorf("unexpected telemetry logger type: %T", v))
}

return ctx
}

// WithMockLogger sets a mock telemetry logger in the context. It overrides the
// default logger if it is already set in the context.
func WithMockLogger(ctx context.Context) context.Context {
v := ctx.Value(telemetryLoggerKey)
if v != nil {
panic(fmt.Errorf("telemetry logger already set in the context: %T", v))
}

return context.WithValue(ctx, telemetryLoggerKey, &mockLogger{})
}

func fromContext(ctx context.Context) Logger {
v := ctx.Value(telemetryLoggerKey)
if v == nil {
panic(fmt.Errorf("telemetry logger not found in the context"))

Check failure on line 51 in libs/telemetry/context.go

View workflow job for this annotation

GitHub Actions / lint

fmt.Errorf can be replaced with errors.New (perfsprint)
}

switch vv := v.(type) {
case *defaultLogger:
return vv
case *mockLogger:
return vv
default:
panic(fmt.Errorf("unexpected telemetry logger type: %T", v))
}
}
77 changes: 77 additions & 0 deletions libs/telemetry/context_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package telemetry

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
)

func TestWithDefaultLogger(t *testing.T) {
ctx := context.Background()

// No default logger set
ctx1 := WithDefaultLogger(ctx)
assert.Equal(t, &defaultLogger{}, ctx1.Value(telemetryLoggerKey))

// Default logger already set
assert.PanicsWithError(t, "default telemetry logger already set in the context: *telemetry.defaultLogger", func() {
WithDefaultLogger(ctx1)
})

// Mock logger already set
ctx2 := WithMockLogger(ctx)
assert.NotPanics(t, func() {
WithDefaultLogger(ctx2)
})

// Unexpected logger type
type foobar struct{}
ctx3 := context.WithValue(ctx, telemetryLoggerKey, &foobar{})
assert.PanicsWithError(t, "unexpected telemetry logger type: *telemetry.foobar", func() {
WithDefaultLogger(ctx3)
})
}

func TestWithMockLogger(t *testing.T) {
ctx := context.Background()

// No logger set
ctx1 := WithMockLogger(ctx)
assert.Equal(t, &mockLogger{}, ctx1.Value(telemetryLoggerKey))

// Logger already set
assert.PanicsWithError(t, "telemetry logger already set in the context: *telemetry.mockLogger", func() {
WithMockLogger(ctx1)
})

// Default logger already set
ctx2 := WithDefaultLogger(ctx)
assert.PanicsWithError(t, "telemetry logger already set in the context: *telemetry.defaultLogger", func() {
WithMockLogger(ctx2)
})
}

func TestFromContext(t *testing.T) {
ctx := context.Background()

// No logger set
assert.PanicsWithError(t, "telemetry logger not found in the context", func() {
fromContext(ctx)
})

// Default logger set
ctx1 := WithDefaultLogger(ctx)
assert.Equal(t, &defaultLogger{}, fromContext(ctx1))

// Mock logger set
ctx2 := WithMockLogger(ctx)
assert.Equal(t, &mockLogger{}, fromContext(ctx2))

// Unexpected logger type
type foobar struct{}
ctx3 := context.WithValue(ctx, telemetryLoggerKey, &foobar{})
assert.PanicsWithError(t, "unexpected telemetry logger type: *telemetry.foobar", func() {
fromContext(ctx3)
})
}
22 changes: 22 additions & 0 deletions libs/telemetry/frontend_log.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package telemetry

import "github.com/databricks/cli/libs/telemetry/protos"

// This corresponds to the FrontendLog lumberjack proto in universe.
// FrontendLog is the top-level struct for any client-side logs at Databricks
// regardless of whether they are generated from the CLI or the web UI.
type FrontendLog struct {
// A unique identifier for the log event generated from the CLI.
FrontendLogEventID string `json:"frontend_log_event_id,omitempty"`

Entry FrontendLogEntry `json:"entry,omitempty"`
}

type FrontendLogEntry struct {
DatabricksCliLog DatabricksCliLog `json:"databricks_cli_log,omitempty"`
}

type DatabricksCliLog struct {
CliTestEvent *protos.CliTestEvent `json:"cli_test_event,omitempty"`
BundleInitEvent *protos.BundleInitEvent `json:"bundle_init_event,omitempty"`
}
Loading

0 comments on commit 9012c4c

Please sign in to comment.