feat: raise error when updating a read-only attribute - #1281
Draft
ogenstad wants to merge 3 commits into
Draft
Conversation
Assigning to a read-only attribute previously failed silently: the value was dropped from the mutation payload at save time with no signal to the caller. Setting a read-only attribute now raises ReadOnlyAttributeError, while loading and re-querying read-only values keep working (those go through the constructor / internal population path). infrahubctl object update now rejects a read-only --set field at validation time with a clear message instead of silently no-op'ing.
Deploying infrahub-sdk-python with
|
| Latest commit: |
131e072
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ba9abd53.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://pog-em--chore-raise-error-up.infrahub-sdk-python.pages.dev |
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## stable #1281 +/- ##
==========================================
+ Coverage 84.16% 84.17% +0.01%
==========================================
Files 147 147
Lines 13045 13058 +13
Branches 1930 1932 +2
==========================================
+ Hits 10979 10992 +13
Misses 1503 1503
Partials 563 563
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
…k usage Replace the _value-poking test helper with a _fetched_file_node helper that builds the node through the public constructor data payload (the path a real query uses). Swap the CTL update schema-attribute stubs from MagicMock to SimpleNamespace.
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
1 participant
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.
Why
Updating a read-only attribute via the SDK failed silently: the value was accepted in memory, then quietly dropped from the mutation at save time, so the change never reached the server and the caller got no signal. This makes it easy to believe a value was written when it never was.
Goal: surface the problem at the point of assignment instead of hiding it.
Closes #1248
What changed
Behavioral changes
node.some_readonly_attr.value = "x") now raisesReadOnlyAttributeErrorimmediately, instead of silently discarding the value.infrahubctl object updatenow rejects a read-only field passed via--setwith a clear error at validation time, instead of silently no-op'ing (it previously even printed a false "Updated ..." line).What stayed the same
Attributeconstructor and the internal population path set the backing value directly and bypass the new guard. A re-query builds a fresh node through the constructor, so read-only data always repopulates.Implementation notes
ReadOnlyAttributeErrorexception.Attribute.valuesetter; an internal_set_value()bypass is used by the two pool re-population sites innode.py._read_only = ["updated_at", "is_inherited"]list that was defined but never referenced.How to review
Start with
infrahub_sdk/node/attribute.py(the setter guard +_set_valuebypass), theninfrahub_sdk/node/node.py(the two internal call sites), theninfrahub_sdk/ctl/object/update.py(validation-time rejection). Tests follow.How to test
Impact & rollout
ReadOnlyAttributeError. This is called out prominently in the changelog fragment. Callers must stop writing to read-only attributes.Follow-up (out of scope here)
tests/unit/ctl/object/test_update.pyis built onunittest.mock(MagicMock/AsyncMock/patch), which we are trying to move away from. This PR minimizes its footprint (schema-attribute stubs useSimpleNamespace, not MagicMock) but the new read-only rejection test still relies on the file's existing mock-based CLI harness. A separate, unrelated change should move this file (and its siblings) off MagicMock toward real schema fixtures.Checklist
changelog/1248.changed.md, flagged as potentially breaking)