direct: detect stale dashboard publishes via publish/update timestamps - #6119
Merged
Conversation
DoRead computed a dashboard's published state as "does GetPublished succeed", but once a dashboard is published GetPublished keeps returning the previous revision even after the draft is updated, so that check cannot tell whether the published content is current. Compare the published revision_create_time against the draft update_time instead: published is true only when GetPublished succeeds AND revision_create_time >= update_time. When the draft was updated without a republish, published becomes false, which differs from the desired true and the planner schedules a republish. Verified on AWS, GCP, Azure and dogfood: a publish always leaves revision_create_time >= update_time, and a draft update pushes update_time ahead until the next publish (so there is no perpetual-republish risk). The draft etag, by contrast, does not reliably change on a display-name update on cloud, so it cannot be used for this. The testserver now emits strictly-increasing RFC3339Nano timestamps (nextTimestamp) so update/publish ordering is deterministic. published is also excluded from config-remote-sync as an internal state field. Co-authored-by: Isaac
Collaborator
Integration test reportCommit: 50ea47d
8 interesting tests: 4 RECOVERED, 4 SKIP
Top 3 slowest tests (at least 2 minutes):
|
… update
Deploys a dashboard, makes an out-of-band draft update without republishing, and
asserts `bundle plan` reports the dashboard's published state needs an update
(published: {old: true, remote: false}). Before the timestamp-based fix, DoRead
reported published=true (GetPublished still returns the previous revision), so
the stale publish was missed and the plan showed no change.
Direct engine only: published is a direct-engine state field.
Co-authored-by: Isaac
janniklasrose
approved these changes
Aug 3, 2026
janniklasrose
left a comment
janniklasrose
left a comment
Contributor
There was a problem hiding this comment.
add a changelog entry
Replace the inline `bundle summary | jq` + manual ACC_REPLS append with read_id.py, which reads the id from state, prints it, and registers the [DASHBOARD1_ID] repl in one step. Goldens are unchanged. Co-authored-by: Isaac
denik
enabled auto-merge
August 3, 2026 09:59
read_id.py cannot be used in republish-after-draft-update because the test runs with MSYS_NO_PATHCONV=1, which mangles the helper's own script path on Windows Git Bash. Restore the inline id capture and note why. Co-authored-by: Isaac
The inherited MSYS_NO_PATHCONV=1 is only needed for scripts that pass leading-slash paths to the CLI; this one does not. Unset it (matching the sibling dashboard tests) so read_id.py resolves its own script path on Windows, and drop the now-redundant test.toml override. Co-authored-by: Isaac
The `Ignore` key sat under the `[EnvMatrix]` header, so TOML parsed it as `EnvMatrix.Ignore` (a bogus matrix variable) rather than the top-level `Ignore`. It was redundant anyway: the parent bundle/test.toml already ignores databricks.yml. Co-authored-by: Isaac
A top-level key placed below a [EnvMatrix] header is silently parsed as EnvMatrix.<key> (a bogus matrix variable), leaving the real field unset. Document the dotted form as the safe idiom. Co-authored-by: Isaac
denik
added a commit
that referenced
this pull request
Aug 5, 2026
Now that DoRead derives published from the publish/update timestamps (#6119), the reason DoUpdate avoided saving state before the publish is gone: a stale publish reports remote published=false, so the remote_already_set skip that would have stranded it can no longer happen. Save the post-update etag and published=false before publishing. A failed publish is now recoverable with a plain deploy: the next plan sees desired published=true against a remote reported as false and republishes. Saving the post-update etag also keeps state in sync with remote, so CheckDashboardsModifiedRemotely no longer misreports this as an out-of-band edit and --force is no longer required. Drops the Badness marker from publish-failure-stale-content and updates it to assert recovery on a plain re-deploy. Co-authored-by: Isaac
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
3 participants
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.
Changes
Detect stale dashboard publishes in the direct engine by comparing the published
revision_create_timeagainst the draftupdate_time, instead of treating "apublished version exists" as "the published content is current".
A dashboard is deployed as a draft update followed by a separate publish. Once
published,
GetPublishedkeeps returning the previous revision even after thedraft changes, so the old
published = (GetPublished succeeds)check could nottell that the published content was stale. Now
publishedis true only whenrevision_create_time >= update_time; a draft updated without a republish readsas
published=falseand the planner surfaces a pending republish.Why not the draft etag
Verified on AWS, GCP and Azure: the draft etag tracks the serialized dashboard
content, so a metadata-only draft change (e.g.
display_name) with an unchangedserialized payload does not bump the etag — while
update_timestill advances.So an etag-based "did the draft change since we published" check would miss that
case; the
revision_create_timevsupdate_timecomparison does not. A publishalways leaves
revision_create_time >= update_time, so there is noperpetual-republish risk.
Scope
This improves detection only. Recovering a stale publish via a plain re-deploy
also requires persisting state before the publish call (so the etag-drift
"modified remotely" guard does not block); that is a separate change. Here, a
stale publish is detected in the plan but recovery may still require
--force(unchanged from today).
Tests
New regression test
republish-after-draft-update: an out-of-band draft updatemakes
bundle planreportpublishedneeds an update (it did not before). Goldensupdated for
detect-changeandpublish-failure-stale-content. The testserver nowemits strictly-increasing timestamps so update/publish ordering is deterministic.