Uh oh!
There was an error while loading. Please reload this page.
testserver: round-trip all catalog fields on create and update - #6158
Merged
Conversation
CreateCatalog accepts connection_name, custom_max_retention_hours and managed_encryption_settings, but the fake server dropped them from the response it stored. The CLI then read back a catalog missing fields it had just sent, so the next plan saw a change that does not exist.
Collaborator
Integration test reportCommit: 079c2e9
10 interesting tests: 5 RECOVERED, 3 SKIP, 2 flaky
Top 50 slowest tests (at least 2 minutes):
|
CatalogsUpdate had the same gap the create handler did: it applied only comment, owner and new_name, so custom_max_retention_hours, managed_encryption_settings, options and properties silently reverted to their create-time values. Changing any of them on a deployed catalog produced drift that never converged. Add a catalog config to the no_drift invariant so the phantom-change symptom is guarded where users hit it, and assert in the unit test that the create request still covers every field CreateCatalog accepts, so a new SDK field fails there rather than as unexplained drift.
radakam
marked this pull request as ready for review
August 4, 2026 15:24
andrewnester
approved these changes
Aug 5, 2026
Uh oh!
There was an error while loading. Please reload this page.
radakam added a commit
that referenced
this pull request
Aug 5, 2026
…ty-resource-names Conflicts in the catalog fake: main (#6158) widened the create/update field set while this branch added the empty-name rejection. Both are kept.
eng-dev-ecosystem-bot
commented
Aug 5, 2026
Collaborator
Integration test reportCommit: 5a9eba2
211 interesting tests: 187 MISS, 17 FAIL, 5 RECOVERED, 2 SKIP
Top 50 slowest tests (at least 2 minutes):
|
eng-dev-ecosystem-bot
commented
Aug 5, 2026
Collaborator
Integration test reportCommit: 5a9eba2
469 interesting tests: 426 MISS, 38 FAIL, 4 RECOVERED, 1 SKIP
Top 50 slowest tests (at least 2 minutes):
|
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 freeto 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
CatalogsCreatenow echoesconnection_name,custom_max_retention_hoursandmanaged_encryption_settings.CatalogsUpdatenow appliescustom_max_retention_hours,managed_encryption_settings,optionsandproperties.Why
RemapStatereads all of these back offCatalogInfo, so any field the fake dropped showed up as a change on the next plan and never converged.Found by fuzz testing.
Tests
New
no_driftconfigcatalog_optional_fields.yml.tmplcoverscustom_max_retention_hoursandmanaged_encryption_settings.connection_nameand the update path are covered by unit tests only —no_driftdeploys once, so no acceptance test exercises the update round-trip.