Trigger recreate for immutable Lakebase database fields#5473
Merged
Conversation
The direct engine attempted in-place updates that the Database API can't honor: database_instances has no rename endpoint, and database_catalogs and synced_database_tables have no update endpoint at all (their Update* SDK methods are stubs the backend rejects with 501 NOT_IMPLEMENTED). Mark the affected fields recreate_on_changes so a change goes through delete + create: name for database_instances, and all settable fields for database_catalogs and synced_database_tables (whose now-dead DoUpdate methods are removed). The testserver catalog/synced-table PATCH handlers return the real 501. A new test asserts every no-DoUpdate resource classifies every settable field, plus direct-engine recreate acceptance tests. Co-authored-by: Isaac
Collaborator
Integration test reportCommit: 02dd7ff
24 interesting tests: 15 SKIP, 7 KNOWN, 2 flaky
Top 26 slowest tests (at least 2 minutes):
|
janniklasrose
approved these changes
Jun 17, 2026
The merge left two entries each for database_instances, database_catalogs, and synced_database_tables in resources.yml: provided_id_fields from main and recreate_on_changes from this branch. yaml.Unmarshal rejects duplicate mapping keys, so MustLoadConfig panicked at init on every direct-engine deploy. Merge each pair into one entry. name is now classified only via provided_id_fields (classifyIDField already plans Recreate on change, and runs before the recreate_on_changes check), so the explicit recreate_on_changes name entries were redundant. database_instances keeps just provided_id_fields; the other two keep recreate_on_changes for their remaining immutable fields. Count provided_id_fields as covered in TestNoUpdateResourcesCoverAllFields, since a change to such a field recreates rather than planning an unsupported update. Co-authored-by: Isaac
Collaborator
Integration test reportCommit: e941e31
507 interesting tests: 461 MISS, 29 FAIL, 7 KNOWN, 4 PANIC, 4 flaky, 2 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 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
Direct engine: recreate (delete + create) instead of attempting an unsupported in-place update for three Lakebase resources.
database_instances: marknameasrecreate_on_changes(the Database API has no rename endpoint).database_catalogsandsynced_database_tables: removeDoUpdateand mark all settable fieldsrecreate_on_changes(these resources have no update endpoint at all).PATCHhandlers now return the real501 NOT_IMPLEMENTEDinstead of faking a successful update.TestNoUpdateResourcesCoverAllFieldsasserts that every resource withoutDoUpdateclassifies every settable field, so therecreate_on_changeslists stay exhaustive as the SDK adds fields.acceptance/bundle/resources/{database_instances,database_catalogs,synced_database_tables}/recreate/.Why
Renaming a
database_instance, or changing any field on adatabase_catalog/synced_database_table, failed at deploy with a confusing error:nameis immutable, and the catalog/synced-tableUpdate*SDK methods are stubs that the backend rejects with501 NOT_IMPLEMENTED(the public API only exposes Create/Get/Delete; the methods exist solely for Terraform). Reproduced live on a workspace. Since these resources have no working update path, the correct behavior is to recreate.Tests
recreateacceptance tests assert the plan showsrecreateand the requests areDELETE+POST(noPATCH); verified thedatabase_instancesone fails without the fix, reproducing the original404.TestNoUpdateResourcesCoverAllFieldsverified to flag fields removed from the recreate lists.basic/single-instance/permissions tests, the invariantno_driftsuite, anddresources+testserverunit tests pass.This pull request and its description were written by Isaac.