Skip to content

Fix never-converging job diff on reordered webhook notifications - #6060

Open
radakam wants to merge 3 commits into
mainfrom
fix-job-webhook-notification-order
Open

Fix never-converging job diff on reordered webhook notifications#6060
radakam wants to merge 3 commits into
mainfrom
fix-job-webhook-notification-order

Conversation

@radakam

@radakam radakam commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary

The Jobs API treats webhook_notifications.on_* destination lists as unordered sets and may return them in a different order than submitted. The direct engine diffed these lists positionally (structdiff with no key func), so a stable server-side reorder produced a phantom diff that bundle plan / bundle deploy could never converge past (1 to change forever). This reproduces after bundle deployment migrate, and on any direct deploy where the remote returns a different-but-valid permutation.

Fix

Register the job-, task-, and for_each_task-level webhook_notifications.on_* paths in ResourceJob.KeyedSlices(), keyed by id, so the lists are compared as sets rather than by index — the same pattern already used for depends_on.

Tests

  • Acceptance test jobs/webhook-reorder-remote: deploys a job, reorders the remote lists via edit_resource.py, and asserts bundle plan reports 0 to change. Verified it fails (1 to change) with the fix removed.
  • Unit test covering the job, task, and nested for_each_task webhook paths.

radakam added 2 commits July 24, 2026 09:58
The Jobs API treats webhook_notifications.on_* lists as unordered sets
and may return them in a different order than submitted. The direct
engine compared these lists positionally (structdiff with no key func),
so a stable server-side reordering produced a phantom diff that
"bundle plan"/"bundle deploy" could never converge past.

Add the job-, task-, and for_each-task-level webhook_notifications.on_*
paths to ResourceJob.KeyedSlices(), keyed by id, so the diff is
order-insensitive. This matches the existing pattern for depends_on.
Comment thread bundle/direct/dresources/job_test.go
Deploys a job with webhook_notifications.on_* destinations, then uses
edit_resource.py to reorder the remote lists (simulating the Jobs API
returning them in a different order) and asserts bundle plan reports
0 to change. Without the KeyedSlices fix this plan shows 1 to change.
@eng-dev-ecosystem-bot

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: 0078d4b

Run: 30086148379

Env 🔄​flaky 💚​RECOVERED 🙈​SKIP ✅​pass 🙈​skip Time
🔄​ aws linux 1 4 4 321 1060 12:38
💚​ aws windows 4 4 324 1058 8:37
💚​ azure linux 4 4 322 1059 12:18
💚​ azure windows 4 4 324 1057 7:41
🔄​ gcp linux 1 1 5 320 1061 13:03
💚​ gcp windows 1 5 323 1059 9:45
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
🔄​ TestSyncIncrementalFileSync ✅​p ✅​p ✅​p ✅​p 🔄​f ✅​p
🔄​ TestSyncIncrementalSyncPythonNotebookToFile 🔄​f ✅​p ✅​p ✅​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 18 slowest tests (at least 2 minutes):
duration env testname
5:33 gcp windows TestAccept
4:10 aws linux TestAccept/bundle/deploy/mlops-stacks/DATABRICKS_BUNDLE_ENGINE=direct
4:09 aws windows TestAccept/bundle/deploy/mlops-stacks/DATABRICKS_BUNDLE_ENGINE=terraform
4:05 gcp linux TestAccept/bundle/deploy/mlops-stacks/DATABRICKS_BUNDLE_ENGINE=terraform
3:34 aws windows TestAccept
3:22 gcp windows TestImportDirDoesNotOverwrite
3:22 azure windows TestAccept
3:05 aws linux TestFilerWorkspaceFilesExtensionsDelete
3:04 gcp linux TestFilerWorkspaceFilesExtensionsDelete
2:41 gcp linux TestFilerWorkspaceFilesExtensionsReadDir
2:28 azure windows TestAccept/bundle/deploy/mlops-stacks/DATABRICKS_BUNDLE_ENGINE=direct
2:22 aws linux TestAccept/bundle/deploy/mlops-stacks/DATABRICKS_BUNDLE_ENGINE=terraform
2:14 aws linux TestFilerWorkspaceFilesExtensionsRead
2:14 azure linux TestAccept/bundle/deploy/mlops-stacks/DATABRICKS_BUNDLE_ENGINE=terraform
2:11 gcp linux TestFilerWorkspaceFilesExtensionsStat
2:04 aws windows TestFilerRecursiveDelete/workspace_files
2:03 gcp linux TestFilerWorkspaceFilesExtensionsRead
2:02 aws linux TestFilerWorkspaceNotebook/sqlNb.sql

@radakam
radakam marked this pull request as ready for review July 24, 2026 11:04
@github-actions

Copy link
Copy Markdown
Contributor

Approval status: pending

/acceptance/bundle/ - needs approval

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

/bundle/ - needs approval

Files: bundle/direct/dresources/job.go, bundle/direct/dresources/job_test.go
Suggested: @denik
Also eligible: @shreyas-goenka, @pietern, @janniklasrose, @andrewnester, @anton-107, @lennartkats-db

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

@radakam
radakam requested a review from denik July 24, 2026 11:49
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.

3 participants