Repository navigation
testserver: match cloud responses for trashed, purged and unknown alerts - #7509
Merged
Merged
Conversation
Co-authored-by: Isaac <no-reply@databricks.com>
github-actions
Bot
added
the
test-only-fix
Contributor
|
Commit: Regression test reportSelected 1 changed tests (limit=20, 0 not selected) 1 tests: regression=0 unreleased=0 coverage=1 failing=0
|
Collaborator
✅ Integration test reportCommit: 3ba8824
Top 6 slowest tests (at least 2 minutes):
|
denik
marked this pull request as ready for review
November 11, 2026 21:05
denik
enabled auto-merge
November 11, 2026 21:05
andrewnester
self-requested a review
November 12, 2026 08:58
andrewnester
approved these changes
Nov 12, 2026
|
|
||
| title "trashed alert is still readable" | ||
| trace $CLI alerts-v2 trash-alert $alert_id | ||
| trace $CLI alerts-v2 get-alert $alert_id | jq -r .lifecycle_state |
Contributor
There was a problem hiding this comment.
The golden shows a bare DELETED; projecting with jq '{lifecycle_state}' like alerts/with_file/script:31 labels the value. Worth matching?
Contributor
Author
There was a problem hiding this comment.
Agreed. #7546 switches it to jq '{lifecycle_state}', the same as alerts/with_file/script, so the golden now shows "lifecycle_state": "DELETED".
AI agent (harness_auto, isaac_claude, opus5.5)
| @@ -0,0 +1,27 @@ | |||
| # Alert get/update/trash responses for trashed, purged and unknown alerts. | |||
| # Expected output was recorded against AWS on 2026-11-11; the test is local-only | |||
| # because the messages are backend-owned and have changed before. | |||
Contributor
There was a problem hiding this comment.
Against the cloud the output differs in two known places (trash-again UUID, timestamp suffix on the file name); can this comment name them so nobody flips Cloud = true blind?
Contributor
Author
There was a problem hiding this comment.
Done in #7546. The header comment now lists both differences: trash-again reports a different UUID, and update on a trashed alert names the file myalert-<timestamp>.dbalert.json.
AI agent (harness_auto, isaac_claude, opus5.5)
tedjt
pushed a commit
to tedjt/cli
that referenced
this pull request
Nov 12, 2026
…es (databricks#7546) ## Changes Follow-up to [databricks#7509](databricks#7509), addressing review comments that arrived after it merged. Both are in `acceptance/cmd/workspace/alerts-v2-not-found`: - The trashed-alert `get-alert` now projects `jq '{lifecycle_state}'`, matching `alerts/with_file/script`, so the golden labels the value instead of printing a bare `DELETED`. - The header comment now names the two known differences on the cloud: trash-again reports a different UUID than the alert id, and update on a trashed alert names the file with a timestamp suffix. That way nobody flips `Cloud = true` without knowing about them. ## Why Review feedback on [databricks#7509](databricks#7509). ## Tests `go test ./acceptance -run TestAccept/cmd/workspace/alerts-v2-not-found` passes locally. ## Dashboard: Follow-up only. No failing test is fixed here. Related: [error/3324](http://go/cli/dashboard/error/3324?main=1&pr=1&end=2026-11-11T17:00:00Z&last=2d) (`Error: Tree node with id [ALERT_ID] does not exist`). This pull request and its description were written by Isaac. Co-authored-by: Isaac <no-reply@databricks.com>
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
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
libs/testservernow answers alert get/update/trash the way the cloud does for trashed, purged and unknown alerts:lifecycle_state: DELETEDINVALID_PARAMETER_VALUECannot move node '<file>' (ID: <id>) because it is in trash folderBAD_REQUESTAlert <id> is already trashed(purge still succeeds)NOT_FOUNDTree node with id <id> does not existNOT_FOUNDAlert with id <id> not found or has no folder node internal nameNOT_FOUNDAlert with id <id> not foundBefore this, get returned 404 with the old
Alert with ID '<id>' does not exist.message for both trashed and missing alerts, and update/trash returned a bare 404 with no body. Update and trash of a trashed alert succeeded.New local test
acceptance/cmd/workspace/alerts-v2-not-foundrecords this matrix.Why
Follow-up to #7492, where the review asked to fix the testserver as well. That PR left the old message in place.
Decisions:
EQUAL) is out of scope.Tests
go test ./acceptance -run TestAccept/cmd/workspace/alerts-v2-not-foundpasses locally.bundle/migrate/*,bundle/state/*andbundle/user_agent/simplefailures show up on a clean main in this environment, so they are unrelated.Cloud = trueviatestme-awsto capture the responses above.Dashboard:
Follow-up to error/3324. It does not fix a failing test, because #7492 already made the cloud test pass:
Error: Tree node with id [ALERT_ID] does not existThis pull request and its description were written by Isaac.