From a1dbbfbeb0ddc0503d35a4fe2a8480291f1925a6 Mon Sep 17 00:00:00 2001 From: pavloKozlov Date: Mon, 3 Aug 2026 11:07:00 +0200 Subject: [PATCH 1/5] bundle/direct: normalize integral doubles when reading DMS state DMS round-trips recorded resource state through a protobuf Struct, whose only numeric type is double, so integers are served back fractional (e.g. "max_concurrent_runs": 1.0). The typed resource structs unmarshal those fields as int and reject the fractional form, so any deploy that reads existing state back from DMS - i.e. every deploy after the first - failed with: cannot plan resources.jobs.daily_sales_rollup: interpreting state: unmarshalling into *jobs.JobSettings: json: cannot unmarshal string into Go value of type int (The "string" in the message is the SDK unmarshaller's quote-and-retry fallback kicking in after the numeric parse fails.) Normalize integral doubles back to integers at the single DMS read boundary in fetchDeploymentResources, before the state reaches the typed resource structs. Genuinely fractional numbers are left untouched. Co-authored-by: Isaac --- bundle/direct/dstate/dms.go | 68 +++++++++++++++++++++++++++++++- bundle/direct/dstate/dms_test.go | 41 +++++++++++++++++++ 2 files changed, 108 insertions(+), 1 deletion(-) diff --git a/bundle/direct/dstate/dms.go b/bundle/direct/dstate/dms.go index 094f114a0a6..a7d172700a6 100644 --- a/bundle/direct/dstate/dms.go +++ b/bundle/direct/dstate/dms.go @@ -1,10 +1,12 @@ package dstate import ( + "bytes" "context" "encoding/json" "errors" "fmt" + "math" "github.com/databricks/cli/bundle/deployplan" "github.com/databricks/cli/libs/log" @@ -82,11 +84,75 @@ func fetchDeploymentResources(ctx context.Context, client bundledeployments.Bund } } + // DMS stores state in a protobuf Struct, whose only numeric type is + // double, so integers come back as "1.0". The typed resource structs + // (e.g. jobs.JobSettings.MaxConcurrentRuns, num_workers) unmarshal those + // fields as int and reject the fractional form, so restore the integral + // doubles to integers before the state reaches them. + state, err := normalizeIntegralNumbers(recorded.State) + if err != nil { + return nil, fmt.Errorf("interpreting state recorded for %s: %w", key, err) + } + out[key] = ResourceEntry{ ID: res.ResourceId, - State: recorded.State, + State: state, DependsOn: recorded.DependsOn, } } return out, nil } + +// normalizeIntegralNumbers rewrites JSON numbers that have no fractional part +// (e.g. "1.0") as integers ("1"). DMS round-trips state through a protobuf +// Struct whose only numeric type is double, so every integer it stores comes +// back fractional; the typed resource structs unmarshal integer fields as int +// and reject that form. Genuinely fractional numbers are left untouched. +// +// A nil or empty input is returned unchanged so an unrecorded resource keeps +// its nil state rather than becoming "null". +func normalizeIntegralNumbers(raw json.RawMessage) (json.RawMessage, error) { + if len(raw) == 0 { + return raw, nil + } + + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.UseNumber() + var v any + if err := dec.Decode(&v); err != nil { + return nil, err + } + + return json.Marshal(normalizeValue(v)) +} + +// normalizeValue walks a decoded JSON value (with numbers as json.Number) and +// converts every integral number to an int64, recursing into objects and +// arrays. Non-numeric leaves are returned as-is. +func normalizeValue(v any) any { + switch t := v.(type) { + case map[string]any: + for k, val := range t { + t[k] = normalizeValue(val) + } + return t + case []any: + for i, val := range t { + t[i] = normalizeValue(val) + } + return t + case json.Number: + // An integer already parses as int64; keep it. Otherwise the value is a + // double, and only its integral form needs rewriting - a real fraction + // must survive untouched (e.g. a float-typed config field). + if i, err := t.Int64(); err == nil { + return i + } + if f, err := t.Float64(); err == nil && f == math.Trunc(f) && !math.IsInf(f, 0) { + return int64(f) + } + return t + default: + return v + } +} diff --git a/bundle/direct/dstate/dms_test.go b/bundle/direct/dstate/dms_test.go index 35fe7acbb0c..0ac5dd8d27d 100644 --- a/bundle/direct/dstate/dms_test.go +++ b/bundle/direct/dstate/dms_test.go @@ -66,3 +66,44 @@ func TestFetchDeploymentResourcesRejectsMalformedState(t *testing.T) { _, err := fetchDeploymentResources(t.Context(), f, "dep-1") assert.ErrorContains(t, err, "interpreting state recorded for resources.jobs.foo") } + +func TestFetchDeploymentResourcesNormalizesIntegralDoubles(t *testing.T) { + // DMS serializes state through a protobuf Struct, so integers come back as + // doubles ("1.0"). The typed job state unmarshals those fields as int, so + // they must be restored to integers on the way out. + recorded := json.RawMessage(`{"state":{"max_concurrent_runs":1.0,"tasks":[{"new_cluster":{"num_workers":2.0}}],"timeout_seconds":0.0}}`) + f := &fakeResourceLister{resources: []bundledeployments.Resource{ + {ResourceKey: "jobs.foo", ResourceId: "123", State: &recorded}, + }} + + got, err := fetchDeploymentResources(t.Context(), f, "dep-1") + require.NoError(t, err) + assert.Equal(t, json.RawMessage(`{"max_concurrent_runs":1,"tasks":[{"new_cluster":{"num_workers":2}}],"timeout_seconds":0}`), got["resources.jobs.foo"].State) +} + +func TestNormalizeIntegralNumbers(t *testing.T) { + tests := []struct { + name string + in string + want string + }{ + {"integral doubles become ints", `{"a":1.0,"b":2.0}`, `{"a":1,"b":2}`}, + {"fractions are preserved", `{"a":1.5,"b":0.25}`, `{"a":1.5,"b":0.25}`}, + {"nested objects and arrays", `{"tasks":[{"n":1.0},{"n":2.5}]}`, `{"tasks":[{"n":1},{"n":2.5}]}`}, + {"large integral double", `{"id":1000000000000000.0}`, `{"id":1000000000000000}`}, + {"non-numbers untouched", `{"s":"x","b":true,"z":null}`, `{"b":true,"s":"x","z":null}`}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := normalizeIntegralNumbers(json.RawMessage(tc.in)) + require.NoError(t, err) + assert.JSONEq(t, tc.want, string(got)) + }) + } +} + +func TestNormalizeIntegralNumbersEmptyInputUnchanged(t *testing.T) { + got, err := normalizeIntegralNumbers(nil) + require.NoError(t, err) + assert.Nil(t, got) +} From b3439863a0e4bb207874821a37188e1999494484 Mon Sep 17 00:00:00 2001 From: pavloKozlov Date: Mon, 3 Aug 2026 11:25:26 +0200 Subject: [PATCH 2/5] libs/dms: send previous_version_id when creating a deployment version Recording a second version for an existing DMS deployment failed: Error: failed to create deployment version: previous_version_id is outdated; the deployment's most recent version is 1. Retry with previous_version_id 1. (400 INVALID_PARAMETER_VALUE) The server uses previous_version_id for optimistic concurrency, but the recorder never set it. The field was also absent from the SDK's bundledeployments.Version until v0.166.0, so bump the SDK to pick it up and set it to the deployment's current last_version_id (empty for the first version, which has no predecessor). The SDK bump also drops the now server-assigned DeploymentId field from CreateDeploymentRequest; regenerate the bundle-deployments command accordingly (create-deployment no longer takes a positional ID). Co-authored-by: Isaac --- .../bundle-deployments/bundle-deployments.go | 15 ++++----------- go.mod | 2 +- go.sum | 4 ++-- libs/dms/recorder.go | 18 +++++++++++++----- libs/dms/recorder_test.go | 7 ++++++- 5 files changed, 26 insertions(+), 20 deletions(-) diff --git a/cmd/workspace/bundle-deployments/bundle-deployments.go b/cmd/workspace/bundle-deployments/bundle-deployments.go index 61f57e00173..3b1edd2bee0 100644 --- a/cmd/workspace/bundle-deployments/bundle-deployments.go +++ b/cmd/workspace/bundle-deployments/bundle-deployments.go @@ -180,26 +180,21 @@ func newCreateDeployment() *cobra.Command { cmd.Flags().StringVar(&createDeploymentReq.Deployment.InitialParentPath, "initial-parent-path", createDeploymentReq.Deployment.InitialParentPath, `The workspace path of the folder where the deployment is initially created.`) // TODO: complex arg: workspace_info - cmd.Use = "create-deployment DEPLOYMENT_ID" + cmd.Use = "create-deployment" cmd.Short = `Create a deployment.` cmd.Long = `Create a deployment. Creates a new deployment in the workspace. - The caller must provide a deployment_id which becomes the final component of - the deployment's resource name. If a deployment with the same ID already - exists, the server returns ALREADY_EXISTS. - - Arguments: - DEPLOYMENT_ID: The ID to use for the deployment, which will become the final component of - the deployment's resource name (i.e. deployments/{deployment_id}).` + The caller must set ` + "`" + `initial_parent_path` + "`" + `. Other fields are ignored on input + and populated by the service.` cmd.Annotations = make(map[string]string) cmd.Annotations["launch_stage"] = "PRIVATE_PREVIEW" cmd.Annotations["launch_stage_display"] = "Private Preview" cmd.Args = func(cmd *cobra.Command, args []string) error { - check := root.ExactArgs(1) + check := root.ExactArgs(0) return check(cmd, args) } @@ -220,8 +215,6 @@ func newCreateDeployment() *cobra.Command { } } } - createDeploymentReq.DeploymentId = args[0] - response, err := w.BundleDeployments.CreateDeployment(ctx, createDeploymentReq) if err != nil { return err diff --git a/go.mod b/go.mod index 7d924904c1f..6a03c1abec3 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/charmbracelet/huh v1.0.0 // MIT github.com/charmbracelet/lipgloss v1.1.0 // MIT github.com/charmbracelet/x/ansi v0.11.7 // MIT - github.com/databricks/databricks-sdk-go v0.160.0 // Apache-2.0 + github.com/databricks/databricks-sdk-go v0.166.0 // Apache-2.0 github.com/google/jsonschema-go v0.4.3 // MIT github.com/google/uuid v1.6.0 // BSD-3-Clause github.com/gorilla/websocket v1.5.3 // BSD-2-Clause diff --git a/go.sum b/go.sum index c7cb717757c..5ce106414f5 100644 --- a/go.sum +++ b/go.sum @@ -69,8 +69,8 @@ github.com/cyphar/filepath-securejoin v0.4.1 h1:JyxxyPEaktOD+GAnqIqTf9A8tHyAG22r github.com/cyphar/filepath-securejoin v0.4.1/go.mod h1:Sdj7gXlvMcPZsbhwhQ33GguGLDGQL7h7bg04C/+u9jI= github.com/danieljoos/wincred v1.2.3 h1:v7dZC2x32Ut3nEfRH+vhoZGvN72+dQ/snVXo/vMFLdQ= github.com/danieljoos/wincred v1.2.3/go.mod h1:6qqX0WNrS4RzPZ1tnroDzq9kY3fu1KwE7MRLQK4X0bs= -github.com/databricks/databricks-sdk-go v0.160.0 h1:vwgT/11y2vMw41BxcKbUUqarg45lmoEdukk9yYJg5AM= -github.com/databricks/databricks-sdk-go v0.160.0/go.mod h1:C5LNgGe6hGuRrTwoxFmuup3XtQQEaqtq0e+K8IFDIS4= +github.com/databricks/databricks-sdk-go v0.166.0 h1:OrVvXMr6MFf3NXZn7EIddzpDE8E/er1TrLcWeoLtOwU= +github.com/databricks/databricks-sdk-go v0.166.0/go.mod h1:C5LNgGe6hGuRrTwoxFmuup3XtQQEaqtq0e+K8IFDIS4= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= diff --git a/libs/dms/recorder.go b/libs/dms/recorder.go index 6973a09b4b5..696dba72c07 100644 --- a/libs/dms/recorder.go +++ b/libs/dms/recorder.go @@ -149,6 +149,10 @@ func (r *Recorder) CompleteVersion(ctx context.Context, success bool) error { // the server assign the ID; otherwise it reads the existing deployment to // compute the next version number. func (r *Recorder) createDeploymentVersion(ctx context.Context) (versionID string, err error) { + // previousVersionID is the deployment's current most-recent version, which the + // server requires for optimistic concurrency (see CreateVersion below). It + // stays empty for the deployment's first version, which has no predecessor. + var previousVersionID string if r.deploymentID != "" { // A resolved node names the deployment, but its record is created by the // first version, so there may be none yet: a deploy that registered the @@ -164,6 +168,7 @@ func (r *Recorder) createDeploymentVersion(ctx context.Context) (versionID strin if parseErr != nil { return "", fmt.Errorf("failed to parse last_version_id %q: %w", dep.LastVersionId, parseErr) } + previousVersionID = dep.LastVersionId versionID = strconv.FormatInt(lastVersion+1, 10) case errors.Is(getErr, apierr.ErrNotFound), errors.Is(getErr, apierr.ErrResourceDoesNotExist): versionID = "1" @@ -194,15 +199,18 @@ func (r *Recorder) createDeploymentVersion(ctx context.Context) (versionID strin versionID = "1" } - // The server validates that versionID equals last_version_id + 1 and returns - // ABORTED otherwise (e.g. a concurrent deploy already created this version). + // The server validates that previous_version_id matches the deployment's + // current most-recent version and returns INVALID_PARAMETER_VALUE otherwise + // (e.g. a concurrent deploy already recorded a newer version). It is empty for + // the first version, which has no predecessor. version, versionErr := r.svc.CreateVersion(ctx, bundledeployments.CreateVersionRequest{ Parent: "deployments/" + r.deploymentID, VersionId: versionID, Version: bundledeployments.Version{ - CliVersion: build.GetInfo().Version, - VersionType: r.versionType, - TargetName: r.targetName, + CliVersion: build.GetInfo().Version, + VersionType: r.versionType, + TargetName: r.targetName, + PreviousVersionId: previousVersionID, }, }) if versionErr != nil { diff --git a/libs/dms/recorder_test.go b/libs/dms/recorder_test.go index d4c7efaca03..459ed286f0f 100644 --- a/libs/dms/recorder_test.go +++ b/libs/dms/recorder_test.go @@ -85,6 +85,7 @@ func TestRecorderFirstDeployCreatesDeploymentWithServerAssignedID(t *testing.T) require.Len(t, f.versions, 1) assert.Equal(t, "1", f.versions[0].VersionId) assert.Equal(t, "deployments/server-generated-id", f.versions[0].Parent) + assert.Empty(t, f.versions[0].Version.PreviousVersionId) assert.Equal(t, int64(1), r.Version()) require.NoError(t, r.CompleteVersion(t.Context(), true)) @@ -104,10 +105,12 @@ func TestRecorderSubsequentDeployReusesDeploymentAndIncrementsVersion(t *testing require.NoError(t, r.CreateVersion(t.Context())) - // No new deployment is created; the version increments to last_version_id + 1. + // No new deployment is created; the version increments to last_version_id + 1 + // and names its predecessor for the server's concurrency check. assert.Empty(t, f.created) require.Len(t, f.versions, 1) assert.Equal(t, "5", f.versions[0].VersionId) + assert.Equal(t, "4", f.versions[0].Version.PreviousVersionId) assert.Equal(t, "stored-id", r.DeploymentID()) } @@ -141,6 +144,8 @@ func TestRecorderMissingDeploymentRecordStartsAtVersionOne(t *testing.T) { require.Len(t, f.versions, 1) assert.Equal(t, "1", f.versions[0].VersionId) assert.Equal(t, "deployments/stored-id", f.versions[0].Parent) + // The first version has no predecessor. + assert.Empty(t, f.versions[0].Version.PreviousVersionId) } func TestRecorderDestroyDeletesDeploymentOnSuccess(t *testing.T) { From 965ac079c267d7fab31d110e40894372c2f1941d Mon Sep 17 00:00:00 2001 From: pavloKozlov Date: Mon, 3 Aug 2026 12:00:02 +0200 Subject: [PATCH 3/5] bundle/direct: send resource_id when recording a delete with DMS Destroying a bundle with DMS recording enabled failed: Error: recording operation for resources.jobs.daily_sales_rollup with the deployment metadata service: resource_id is required for OPERATION_ACTION_TYPE_DELETE operations (400 INVALID_PARAMETER_VALUE) The delete path recorded the operation with an empty resource ID, but DMS requires resource_id on a DELETE. The ID is available in state, but both Destroy and DeleteState remove it before the record call, so capture it beforehand and pass it through. Co-authored-by: Isaac --- bundle/direct/bundle_apply.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bundle/direct/bundle_apply.go b/bundle/direct/bundle_apply.go index 46d70c7b135..2a540c88bb3 100644 --- a/bundle/direct/bundle_apply.go +++ b/bundle/direct/bundle_apply.go @@ -93,6 +93,10 @@ func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.Workspa } if action == deployplan.Delete { + // Capture the ID before the delete: DMS requires resource_id on a + // DELETE operation, but both Destroy and DeleteState drop it from state, + // so GetResourceID would return empty afterwards. + resourceID := b.StateDB.GetResourceID(resourceKey) if entry.Gone { // Planning confirmed the resource is already deleted remotely; only // remove it from the state, without calling the delete API. @@ -105,7 +109,7 @@ func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.Workspa return false } // Record the delete with DMS. State is nil: the resource is gone. - if err := opQueue.record(ctx, resourceKey, action, "", nil, nil); err != nil { + if err := opQueue.record(ctx, resourceKey, action, resourceID, nil, nil); err != nil { logdiag.LogError(ctx, fmt.Errorf("%s: %w", errorPrefix, err)) return false } From 38b18e43171b1a4a020c7baf9657ee512a60c763 Mon Sep 17 00:00:00 2001 From: pavloKozlov Date: Mon, 3 Aug 2026 12:00:03 +0200 Subject: [PATCH 4/5] bundle/direct: tolerate CreateOperation response deserialization errors Recording an operation intermittently failed the deploy with: Error: recording operation for resources.jobs.daily_sales_rollup with the deployment metadata service: failed to unmarshal response body: invalid character '1' after top-level value DMS serves sequence_id as a JSON string ("1") per proto3 int64 encoding, but bundledeployments.Operation.SequenceId is an int64 the SDK cannot parse from a string (still true as of SDK v0.168.0). It surfaces only on responses that carry sequence_id, so deploys failed at random on a call the server actually accepted. The CLI discards the CreateOperation response, so a failure to deserialize it does not mean the operation was not recorded. The SDK maps status >= 400 to *apierr.APIError before reading the body, so a "failed to unmarshal response body" error means a 2xx: tolerate exactly that, and keep failing on API errors and transport errors (where the request may not have been recorded). Co-authored-by: Isaac --- bundle/direct/oprecorder.go | 19 +++++++++++++++ bundle/direct/oprecorder_test.go | 40 +++++++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/bundle/direct/oprecorder.go b/bundle/direct/oprecorder.go index d0d2694d358..a400b73a250 100644 --- a/bundle/direct/oprecorder.go +++ b/bundle/direct/oprecorder.go @@ -3,11 +3,14 @@ package direct import ( "context" "encoding/json" + "errors" "fmt" "strings" "github.com/databricks/cli/bundle/deployplan" "github.com/databricks/cli/bundle/direct/dstate" + "github.com/databricks/cli/libs/log" + "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/service/bundledeployments" ) @@ -107,6 +110,22 @@ func (r *operationRecorder) upload(ctx context.Context, resourceKey string, op r ResourceKey: dmsKey, Operation: operation, }) + // The CLI discards the response, so a failure to deserialize it does not mean + // the operation was not recorded: DMS serves sequence_id as a JSON string + // ("1") per proto3 int64 encoding, but bundledeployments.Operation.SequenceId + // is an int64 the SDK cannot parse from a string. This surfaces intermittently + // (only responses that carry sequence_id trip it) as a spurious deploy + // failure on a call the server accepted. + // + // The SDK emits this only on the 2xx body-parse path - a status >= 400 is + // mapped to *apierr.APIError before the body is read - so a "failed to + // unmarshal response body" error means the operation was recorded. Tolerate + // exactly that, and nothing broader: a transport error means the request may + // not have reached DMS and must still fail the deploy. + if err != nil && !errors.As(err, new(*apierr.APIError)) && strings.Contains(err.Error(), "failed to unmarshal response body") { + log.Debugf(ctx, "ignoring response deserialization error from CreateOperation for %s (operation was recorded): %v", dmsKey, err) + return nil + } return err } diff --git a/bundle/direct/oprecorder_test.go b/bundle/direct/oprecorder_test.go index 674c78abf77..fad862fe2d4 100644 --- a/bundle/direct/oprecorder_test.go +++ b/bundle/direct/oprecorder_test.go @@ -2,10 +2,12 @@ package direct import ( "context" + "errors" "sync" "testing" "github.com/databricks/cli/bundle/deployplan" + "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/service/bundledeployments" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -16,13 +18,14 @@ type fakeOpClient struct { mu sync.Mutex requests []bundledeployments.CreateOperationRequest + err error } func (f *fakeOpClient) CreateOperation(ctx context.Context, req bundledeployments.CreateOperationRequest) (*bundledeployments.Operation, error) { f.mu.Lock() defer f.mu.Unlock() f.requests = append(f.requests, req) - return &bundledeployments.Operation{}, nil + return &bundledeployments.Operation{}, f.err } // uploadOne records a single operation through the given uploader, mirroring what @@ -52,6 +55,41 @@ func TestOperationRecorderStripsResourcePrefix(t *testing.T) { require.NotNil(t, req.Operation.State) } +func TestOperationRecorderToleratesResponseDeserializationError(t *testing.T) { + // DMS returns sequence_id as a JSON string the SDK cannot parse into its + // int64 field, so CreateOperation can fail to deserialize a response the + // server accepted. The CLI discards the response, so this must not fail the + // deploy. + f := &fakeOpClient{err: errors.New("failed to unmarshal response body: invalid character '1' after top-level value")} + r := NewOperationRecorder(f, "dep-1", 2) + + op, err := newRecordedOperation(deployplan.Create, "job-123", map[string]string{"name": "foo"}, nil) + require.NoError(t, err) + assert.NoError(t, r.upload(t.Context(), "resources.jobs.foo", op)) + assert.Len(t, f.requests, 1) +} + +func TestOperationRecorderPropagatesAPIError(t *testing.T) { + // A real API error (status >= 400) must still fail the deploy. + f := &fakeOpClient{err: &apierr.APIError{StatusCode: 400, ErrorCode: "INVALID_PARAMETER_VALUE", Message: "bad request"}} + r := NewOperationRecorder(f, "dep-1", 2) + + op, err := newRecordedOperation(deployplan.Create, "job-123", map[string]string{"name": "foo"}, nil) + require.NoError(t, err) + assert.Error(t, r.upload(t.Context(), "resources.jobs.foo", op)) +} + +func TestOperationRecorderPropagatesTransportError(t *testing.T) { + // A transport error means the request may never have reached DMS, so it must + // not be swallowed like a response-deserialization error. + f := &fakeOpClient{err: errors.New("dial tcp: connection refused")} + r := NewOperationRecorder(f, "dep-1", 2) + + op, err := newRecordedOperation(deployplan.Create, "job-123", map[string]string{"name": "foo"}, nil) + require.NoError(t, err) + assert.Error(t, r.upload(t.Context(), "resources.jobs.foo", op)) +} + func TestNewRecordedOperationRecordsStateAsIs(t *testing.T) { state := struct { Name string `json:"name"` From e6dc076b5b243d4385641d55d1c6fe4a4f2e1430 Mon Sep 17 00:00:00 2001 From: pavloKozlov Date: Mon, 3 Aug 2026 12:09:34 +0200 Subject: [PATCH 5/5] bundle/direct: record git info on DMS deployment versions The DMS Version carries a GitInfo (origin_url, branch, commit), but the recorder never populated it, so git_info came back empty on every recorded version. The data is already resolved by the LoadGitDetails mutator (run in the initialize phase, before any recorder) into bundle.git.*, or set explicitly by the user under bundle.git in databricks.yml. Map bundle.git onto the version's GitInfo when creating it. Send nil when no git details are known (the bundle is not in a git repository) so DMS records no git info rather than empty strings. Co-authored-by: Isaac --- bundle/phases/dms.go | 19 +++++++++++++++++ libs/dms/recorder.go | 8 +++++-- libs/dms/recorder_test.go | 45 +++++++++++++++++++++++++++++++++------ 3 files changed, 63 insertions(+), 9 deletions(-) diff --git a/bundle/phases/dms.go b/bundle/phases/dms.go index 3d2f4f54009..6f8c594aeb3 100644 --- a/bundle/phases/dms.go +++ b/bundle/phases/dms.go @@ -6,6 +6,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config/engine" "github.com/databricks/cli/libs/dms" + "github.com/databricks/databricks-sdk-go/service/bundledeployments" ) // newDeploymentRecorder returns a dms.Recorder for the current deployment, or @@ -39,5 +40,23 @@ func newDeploymentRecorder(ctx context.Context, b *bundle.Bundle, eng engine.Eng statePath, b.Config.Bundle.Target, versionType, + gitInfo(b), ), nil } + +// gitInfo maps the bundle's resolved git details onto the DMS version's +// GitInfo. The details come from the LoadGitDetails mutator (run in the +// initialize phase, before any recorder), or from user-set bundle.git.* values. +// Returns nil when none are known - the bundle is not in a git repository - so +// DMS records no git info rather than empty strings. +func gitInfo(b *bundle.Bundle) *bundledeployments.GitInfo { + g := b.Config.Bundle.Git + if g.OriginURL == "" && g.Branch == "" && g.Commit == "" { + return nil + } + return &bundledeployments.GitInfo{ + OriginUrl: g.OriginURL, + Branch: g.Branch, + Commit: g.Commit, + } +} diff --git a/libs/dms/recorder.go b/libs/dms/recorder.go index 696dba72c07..ae0bc08a9e8 100644 --- a/libs/dms/recorder.go +++ b/libs/dms/recorder.go @@ -39,6 +39,7 @@ type Recorder struct { statePath string targetName string versionType VersionType + gitInfo *bundledeployments.GitInfo // populated by CreateVersion versionNum int64 @@ -49,14 +50,16 @@ type Recorder struct { // ID resolved from the deployment's workspace node, or empty if this bundle has // not yet recorded a deployment (the server assigns one during CreateVersion). // statePath is the bundle's remote state directory, under which DMS registers -// the deployment node. -func NewRecorder(svc bundledeployments.BundleDeploymentsInterface, deploymentID, statePath, targetName string, versionType VersionType) *Recorder { +// the deployment node. gitInfo is the source's git origin/branch/commit recorded +// on each version, or nil when the bundle is not in a git repository. +func NewRecorder(svc bundledeployments.BundleDeploymentsInterface, deploymentID, statePath, targetName string, versionType VersionType, gitInfo *bundledeployments.GitInfo) *Recorder { return &Recorder{ svc: svc, deploymentID: deploymentID, statePath: statePath, targetName: targetName, versionType: versionType, + gitInfo: gitInfo, } } @@ -211,6 +214,7 @@ func (r *Recorder) createDeploymentVersion(ctx context.Context) (versionID strin VersionType: r.versionType, TargetName: r.targetName, PreviousVersionId: previousVersionID, + GitInfo: r.gitInfo, }, }) if versionErr != nil { diff --git a/libs/dms/recorder_test.go b/libs/dms/recorder_test.go index 459ed286f0f..0ec84fa1934 100644 --- a/libs/dms/recorder_test.go +++ b/libs/dms/recorder_test.go @@ -69,7 +69,7 @@ func (f *fakeDMS) Heartbeat(ctx context.Context, req bundledeployments.Heartbeat func TestRecorderFirstDeployCreatesDeploymentWithServerAssignedID(t *testing.T) { f := &fakeDMS{assignedID: "server-generated-id"} // A first deploy resolves no deployment ID from the workspace. - r := NewRecorder(f, "", testStatePath, "dev", VersionTypeDeploy) + r := NewRecorder(f, "", testStatePath, "dev", VersionTypeDeploy, nil) require.NoError(t, r.CreateVersion(t.Context())) @@ -101,7 +101,7 @@ func TestRecorderSubsequentDeployReusesDeploymentAndIncrementsVersion(t *testing }, } // A subsequent deploy passes the stored deployment ID. - r := NewRecorder(f, "stored-id", testStatePath, "dev", VersionTypeDeploy) + r := NewRecorder(f, "stored-id", testStatePath, "dev", VersionTypeDeploy, nil) require.NoError(t, r.CreateVersion(t.Context())) @@ -114,13 +114,44 @@ func TestRecorderSubsequentDeployReusesDeploymentAndIncrementsVersion(t *testing assert.Equal(t, "stored-id", r.DeploymentID()) } +func TestRecorderRecordsGitInfo(t *testing.T) { + f := &fakeDMS{ + getDeployment: func(id string) (*bundledeployments.Deployment, error) { + return &bundledeployments.Deployment{Name: "deployments/" + id, LastVersionId: "1"}, nil + }, + } + git := &bundledeployments.GitInfo{ + OriginUrl: "https://github.com/pavloKozlov/bundle-test.git", + Branch: "main", + Commit: "e62c64503a29f91af22ec6544ae9610490ad1fce", + } + r := NewRecorder(f, "stored-id", testStatePath, "dev", VersionTypeDeploy, git) + + require.NoError(t, r.CreateVersion(t.Context())) + require.Len(t, f.versions, 1) + assert.Equal(t, git, f.versions[0].Version.GitInfo) +} + +func TestRecorderGitInfoNilWhenAbsent(t *testing.T) { + f := &fakeDMS{ + getDeployment: func(id string) (*bundledeployments.Deployment, error) { + return &bundledeployments.Deployment{Name: "deployments/" + id, LastVersionId: "1"}, nil + }, + } + r := NewRecorder(f, "stored-id", testStatePath, "dev", VersionTypeDeploy, nil) + + require.NoError(t, r.CreateVersion(t.Context())) + require.Len(t, f.versions, 1) + assert.Nil(t, f.versions[0].Version.GitInfo) +} + func TestRecorderGetDeploymentErrorFailsDeploy(t *testing.T) { f := &fakeDMS{ getDeployment: func(id string) (*bundledeployments.Deployment, error) { return nil, errors.New("boom") }, } - r := NewRecorder(f, "stored-id", testStatePath, "dev", VersionTypeDeploy) + r := NewRecorder(f, "stored-id", testStatePath, "dev", VersionTypeDeploy, nil) err := r.CreateVersion(t.Context()) assert.ErrorContains(t, err, "failed to get deployment") @@ -137,7 +168,7 @@ func TestRecorderMissingDeploymentRecordStartsAtVersionOne(t *testing.T) { return nil, fmt.Errorf("deployment: %w", apierr.ErrNotFound) }, } - r := NewRecorder(f, "stored-id", testStatePath, "dev", VersionTypeDeploy) + r := NewRecorder(f, "stored-id", testStatePath, "dev", VersionTypeDeploy, nil) require.NoError(t, r.CreateVersion(t.Context())) assert.Empty(t, f.created) @@ -154,7 +185,7 @@ func TestRecorderDestroyDeletesDeploymentOnSuccess(t *testing.T) { return &bundledeployments.Deployment{Name: "deployments/" + id, LastVersionId: "2"}, nil }, } - r := NewRecorder(f, "stored-id", testStatePath, "dev", VersionTypeDestroy) + r := NewRecorder(f, "stored-id", testStatePath, "dev", VersionTypeDestroy, nil) require.NoError(t, r.CreateVersion(t.Context())) assert.Equal(t, bundledeployments.VersionTypeVersionTypeDestroy, f.versions[0].Version.VersionType) @@ -170,7 +201,7 @@ func TestRecorderFailedDestroyKeepsDeployment(t *testing.T) { return &bundledeployments.Deployment{Name: "deployments/" + id, LastVersionId: "2"}, nil }, } - r := NewRecorder(f, "stored-id", testStatePath, "dev", VersionTypeDestroy) + r := NewRecorder(f, "stored-id", testStatePath, "dev", VersionTypeDestroy, nil) require.NoError(t, r.CreateVersion(t.Context())) require.NoError(t, r.CompleteVersion(t.Context(), false)) @@ -190,7 +221,7 @@ func TestNilRecorderIsNoOp(t *testing.T) { func TestRecorderCompleteVersionNoOpWithoutCreateVersion(t *testing.T) { f := &fakeDMS{} - r := NewRecorder(f, "stored-id", testStatePath, "dev", VersionTypeDeploy) + r := NewRecorder(f, "stored-id", testStatePath, "dev", VersionTypeDeploy, nil) // CompleteVersion before CreateVersion is a no-op (nothing was claimed). require.NoError(t, r.CompleteVersion(t.Context(), true)) assert.Empty(t, f.completed)