Skip to content

bundle: write config sync key renames as a key rewrite - #6134

Closed
ilyakuz-db wants to merge 1 commit into
configsync/block-scoped-removalfrom
configsync/rename-keyed-elements
Closed

bundle: write config sync key renames as a key rewrite#6134
ilyakuz-db wants to merge 1 commit into
configsync/block-scoped-removalfrom
configsync/rename-keyed-elements

Conversation

@ilyakuz-db

@ilyakuz-db ilyakuz-db commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Stack

PR What it does
1 #6117 Route each change to the one block that defines it
2 #6135 Removing an element defined in several blocks deletes it from each
3 this PR A key change is written as a key rewrite in every defining block

Each PR is based on the one above it and carries the acceptance tests for its own behaviour.

Changes

Writes a remote key change on a keyed list element as a rewrite of that one key, instead of deleting the element and re-emitting it.

A key change (task_key, job_cluster_key, a parameter name) is reported as an unrelated remove plus add — nothing links the two halves. pairRenames matches them back up by identity: a remove and an add on the same sequence whose values are equal apart from the key. The pair is then written from the remove half, which is the one that has a source location, as a Replace of the key field in every block that defines the element.

For an element defined in both the top-level and a targets.<target> block the key is rewritten in both, and every other field stays in the block it was written in:

# remote: task_key shared -> a_shared
 resources: {jobs: {rename_job: {tasks:
-  - task_key: shared
+  - task_key: a_shared
       max_retries: 1}}}                  # stays top-level
 targets: {dev: {resources: {jobs: {rename_job: {tasks:
-      - task_key: shared
+      - task_key: a_shared
           timeout_seconds: 45}}}}}       # stays target-scoped

Two cases need care:

  • The rewrite goes through the same index bookkeeping as every other write. A removal earlier in the same block shifts every later element down, so the rename must be written at the shifted position. Emitting the pre-removal index renames the wrong task and loses the one that was meant to be renamed — in a single-block bundle, with no split lists involved.
  • A key change whose element also had a field edited is left unapplied. The halves cannot be matched by identity, so they would route apart: the removal deletes a split element from every block while the addition recreates it in one, collapsing the split and moving fields such as timeout_seconds out of the target scope into shared configuration. Holding both halves back leaves it for a later run. A plain removal with no matching addition is unaffected.

Why

Recreating the element from the remote value was lossy in three ways, all visible in the goldens this PR updates:

  • field order and quoting were rewrittentask_rename_revert, job_fields, job_multiple_tasks and multiple_files all previously re-emitted the whole task with keys reordered and notebook_path requoted. They now show a one-line key change.
  • a split element collapsed into one block, moving fields out of the scope the user chose.
  • the remove and add halves were routed independently, so a two-block element could be deleted in both places and recreated once, or duplicated.

config-remote-sync runs unattended, so a rename that silently moves a field between scopes, or renames the wrong element, is a wrong write rather than a cosmetic one.

Tests

New split/keyed_rename covers three scenarios, all failing on the parent commit:

  • rename a two-block element and a target-only sibling in one run, with new keys (a_shared, z_solo) whose sort order is the reverse of the block order — an implementation that pairs renames positionally swaps their contents. Asserts the two-block key appears twice, once per block, and the target-only key once.
  • remove one task and rename a later one in the same run, single block: asserts the removed task is gone, the renamed one carries its own fields, and the task after it keeps its name.
  • rename plus a field edit on the same two-block element: asserts the source is unchanged and timeout_seconds stays target-scoped.

Four pre-existing goldens improve to minimal one-line diffs.

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Approval status: pending

/acceptance/bundle/ - needs approval

9 files changed
Suggested: @pietern
Also eligible: @denik, @andrewnester, @anton-107, @janniklasrose, @lennartkats-db, @shreyas-goenka

/bundle/ - needs approval

Files: bundle/configsync/blockindex.go, bundle/configsync/rename.go, bundle/configsync/resolve.go
Suggested: @pietern
Also eligible: @denik, @andrewnester, @anton-107, @janniklasrose, @lennartkats-db, @shreyas-goenka

Any maintainer (@andrewnester, @anton-107, @denik, @pietern, @shreyas-goenka, @simonfaltum, @renaudhartert-db, @janniklasrose, @lennartkats-db) can approve all areas.
See OWNERS for ownership rules.

@ilyakuz-db
ilyakuz-db force-pushed the configsync/block-provenance branch from f957e06 to 7075667 Compare August 2, 2026 12:21
@ilyakuz-db
ilyakuz-db force-pushed the configsync/rename-keyed-elements branch from aa51acf to 94680ae Compare August 2, 2026 12:21
@ilyakuz-db
ilyakuz-db changed the base branch from configsync/block-provenance to configsync/block-scoped-removal August 2, 2026 12:21
@eng-dev-ecosystem-bot

eng-dev-ecosystem-bot commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: 36cc95b

Run: 30768928019

Env 🔄​flaky 💚​RECOVERED 🙈​SKIP ✅​pass 🙈​skip Time
💚​ aws linux 4 4 334 1069 4:27
💚​ aws windows 4 4 324 1071 6:38
🔄​ azure linux 2 4 4 332 1068 4:50
💚​ azure windows 4 4 324 1070 6:28
💚​ gcp linux 1 5 333 1070 4:22
💚​ gcp windows 1 5 323 1072 6:08
10 interesting tests: 4 RECOVERED, 4 SKIP, 2 flaky
Test Name aws linux aws windows azure linux azure windows gcp linux gcp windows
💚​ TestAccept 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R
🙈​ TestAccept/bundle/invariant/no_drift 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/vector_search_endpoints/drift/recreated_same_name 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/vector_search_indexes/recreate/embedding_dimension 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/ssh/connection 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🔄​ TestFilerReadWrite ✅​p ✅​p 🔄​f ✅​p ✅​p ✅​p
🔄​ TestFilerReadWrite/files ✅​p ✅​p 🔄​f ✅​p ✅​p ✅​p
💚​ TestFetchRepositoryInfoAPI_FromRepo 💚​R 💚​R 💚​R 💚​R 🙈​S 🙈​S
💚​ TestFetchRepositoryInfoAPI_FromRepo/root 💚​R 💚​R 💚​R 💚​R
💚​ TestFetchRepositoryInfoAPI_FromRepo/subdir 💚​R 💚​R 💚​R 💚​R
Top 3 slowest tests (at least 2 minutes):
duration env testname
5:20 aws windows TestAccept
5:17 azure windows TestAccept
5:02 gcp windows TestAccept

@ilyakuz-db
ilyakuz-db force-pushed the configsync/block-scoped-removal branch from 88ba6e6 to 2aca20d Compare August 2, 2026 14:01
@ilyakuz-db
ilyakuz-db force-pushed the configsync/rename-keyed-elements branch from 94680ae to 0f2b1bb Compare August 2, 2026 14:01
@ilyakuz-db
ilyakuz-db force-pushed the configsync/block-scoped-removal branch from 2aca20d to af80453 Compare August 2, 2026 14:46
@ilyakuz-db
ilyakuz-db force-pushed the configsync/rename-keyed-elements branch from 0f2b1bb to a65ba01 Compare August 2, 2026 14:46
@ilyakuz-db
ilyakuz-db force-pushed the configsync/block-scoped-removal branch from af80453 to c7d2043 Compare August 2, 2026 18:53
@ilyakuz-db
ilyakuz-db force-pushed the configsync/rename-keyed-elements branch from a65ba01 to e4db4df Compare August 2, 2026 18:53
@ilyakuz-db
ilyakuz-db force-pushed the configsync/block-scoped-removal branch from c7d2043 to add864c Compare August 2, 2026 19:15
@ilyakuz-db
ilyakuz-db force-pushed the configsync/rename-keyed-elements branch from e4db4df to 46ee26e Compare August 2, 2026 19:15
@ilyakuz-db
ilyakuz-db force-pushed the configsync/block-scoped-removal branch from add864c to f1fc0e3 Compare August 2, 2026 19:58
@ilyakuz-db
ilyakuz-db force-pushed the configsync/rename-keyed-elements branch from 46ee26e to 1e50904 Compare August 2, 2026 19:58
@ilyakuz-db
ilyakuz-db force-pushed the configsync/block-scoped-removal branch from f1fc0e3 to 490a0da Compare August 2, 2026 21:52
@ilyakuz-db
ilyakuz-db force-pushed the configsync/rename-keyed-elements branch from 1e50904 to 36cc95b Compare August 2, 2026 21:52
@ilyakuz-db

Copy link
Copy Markdown
Contributor Author

Consolidated into #6138 — the stack is merged into a single PR with the same final tree.

@ilyakuz-db ilyakuz-db closed this Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants