dms: send every field the update mask names - #6372
Draft
shreyas-goenka wants to merge 2 commits into
Draft
Conversation
The sink was the writer's only caller, so the interface bought an indirection and nothing else. Start returns the sink now, which also drops the ctx from StartRecording - the goroutine belongs to whoever created the version. Writes were already serialized by the queue, so the sequence-id map loses its mutex. No sink test goes through the API any more: the queue is driven directly, and what the service sees is asserted by acceptance/bundle/dms - including two new tests, for a delete that fails and for a recording that is rejected once. The first of those found a bug. A failure update claimed no resource_id, so the id it was handed never went out and the service refused the update to a delete operation. KeepsState names the field now: a failure says which resource it failed on, and says so even when there is none to name - a create that never got an id, or a recreate whose delete already dropped it. Co-authored-by: Isaac
UpdateOperation listed error_message in its update_mask but marked the body field omitempty, so a successful apply - which has no error - dropped it from the body while still claiming to write it. The service rejects that: error_message is required when 'error_message' is in update_mask (an empty value clears it) (400 INVALID_PARAMETER_VALUE) Verified against a live staging workspace, where every deploy failed on the first operation. The fake server accepted it because it treated a masked-but-absent field as "set to empty", so it now returns the same 400 and the local suite covers this. Co-authored-by: Isaac <no-reply@databricks.com>
shreyas-goenka
force-pushed
the
isaac/pr6052-fixes
branch
19 times, most recently
from
August 26, 2026 00:53
d7ab4a2 to
435683c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #6094.
UpdateOperationnamederror_messagein itsupdate_maskbut marked the body fieldomitempty, so a successful apply dropped it while still claiming to write it — the service rejects that witherror_message is required when 'error_message' is in update_mask, which failed every deploy against a live staging workspace. The fake server had accepted it by treating a masked-but-absent field as empty, so it now returns the same 400.Note the DMS write path still cannot complete end-to-end:
CreateVersionRequest.operationsis a sibling ofversionwhile the RPC setsbody: "version", so the staged operation plan never reaches the server and eachUpdateOperation404s. That fix is server-side.This pull request and its description were written by Isaac.