Skip to content

Ignore output-only fields via ignore_remote_changes instead of zeroing#5678

Merged
denik merged 3 commits into
mainfrom
denik/do-not-zero
Jun 22, 2026
Merged

Ignore output-only fields via ignore_remote_changes instead of zeroing#5678
denik merged 3 commits into
mainfrom
denik/do-not-zero

Conversation

@denik

@denik denik commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

dashboard and registered_model RemapState zeroed out output-only fields (create_time, update_time, created_at, etc.) so they wouldn't show up in diff computation. This discards the real remote value and duplicates suppression logic.

Carry the real remote values through and rely on ignore_remote_changes instead:

  • dashboard: already covered by resources.generated.yml (OpenAPI output_only annotations)
  • registered_model: already covered by resources.yml

Added a RULE to dresources/README.md documenting this. Regenerated acceptance outputs — the fields now appear as skip / output_only instead of being hidden. Verified with the invariant idempotency test and full acceptance suite.

This pull request and its description were written by Isaac.

dashboard and registered_model RemapState zeroed output-only fields
(create_time, update_time, etc.) to hide them from diff computation.
Carry the real remote values through and rely on ignore_remote_changes
instead, which already covers them (generated config for dashboard,
resources.yml for registered_model).

Co-authored-by: Isaac
@denik denik temporarily deployed to test-trigger-is June 22, 2026 10:42 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is June 22, 2026 10:42 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is June 22, 2026 10:50 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is June 22, 2026 10:50 — with GitHub Actions Inactive
@eng-dev-ecosystem-bot

eng-dev-ecosystem-bot commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: 957a5c9

Run: 27966435816

Env 🟨​KNOWN ✅​pass 🙈​skip Time
🟨​ aws linux 1 216 99 3:07
🟨​ aws windows 1 218 97 2:36
🟨​ aws-ucws linux 1 297 18 3:45
🟨​ aws-ucws windows 1 299 16 3:39
🟨​ azure linux 1 216 98 3:10
🟨​ azure windows 1 218 96 2:32
🟨​ azure-ucws linux 1 299 15 4:03
🟨​ azure-ucws windows 1 301 13 3:26
🟨​ gcp linux 1 215 100 3:08
🟨​ gcp windows 1 217 98 2:38
Test Name aws linux aws windows aws-ucws linux aws-ucws windows azure linux azure windows azure-ucws linux azure-ucws windows gcp linux gcp windows
🟨​ TestAccept 🟨​K 🟨​K 🟨​K 🟨​K 🟨​K 🟨​K 🟨​K 🟨​K 🟨​K 🟨​K

DoUpdate's return value feeds reference resolution (RemoteStateCache),
not diffing. It was zeroing creation_timestamp, last_updated_timestamp,
latest_versions, and user_id even though the UpdateModel response
provides them, so references resolved to empty until the next DoRead.
Carry those through; only Id and PermissionLevel stay empty since
ml.Model does not include them. Add nil guards on the response.

Co-authored-by: Isaac
@denik denik temporarily deployed to test-trigger-is June 22, 2026 16:04 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is June 22, 2026 16:04 — with GitHub Actions Inactive
@denik denik enabled auto-merge June 22, 2026 16:05
@denik denik added this pull request to the merge queue Jun 22, 2026
Merged via the queue into main with commit 330d1c9 Jun 22, 2026
23 checks passed
@denik denik deleted the denik/do-not-zero branch June 22, 2026 17:09
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