Skip to content

Validate properties on assignment#278

Merged
rwb27 merged 7 commits intomainfrom
validate-properties-on-assignment
Apr 1, 2026
Merged

Validate properties on assignment#278
rwb27 merged 7 commits intomainfrom
validate-properties-on-assignment

Conversation

@rwb27
Copy link
Copy Markdown
Collaborator

@rwb27 rwb27 commented Mar 3, 2026

This makes use of the validation logic now added to PropertyInfo to validate properties when they are set from Python. Properties are already validated when set over HTTP.

I've added to existing tests to ensure that we test this in various ways, both by calling __set__ on a mocked Thing and in more realistic contexts.

I have added lt.FEATURE_FLAGS which makes this an opt-in feature. That should mean that we can test against the OFM codebase as-is, and that it should pass when we use a feature branch that enables validation. I can confirm that:

  • This branch passes the OFM test suite as-is, i.e. it's backwards compatible (disabled by default). See the test-against-ofm-v3 job.
  • The feature can be enabled and pass the OFM test suite with minor modifications, detailed in OFM!555. See the test-against-ofm-feature-branch job.

It's slightly unfortunate that this PR now contains two new things (feature flags and validation) but it didn't make sense to merge one without the other. I would like to improve feature flags further, but I will save that for a future PR.

OFM Feature Branch: v3-labthings-validate-properties-on-set

@barecheck
Copy link
Copy Markdown

barecheck bot commented Mar 3, 2026

Barecheck - Code coverage report

Total: 96.38%

Your code coverage diff: 0.03% ▴

Uncovered files and lines
FileLines
src/labthings_fastapi/properties.py505, 803, 807, 830-833, 905, 928, 1183

@rwb27 rwb27 added this to the v0.2.0 milestone Mar 3, 2026
@rwb27 rwb27 force-pushed the validate-properties-on-assignment branch 2 times, most recently from 9f819fd to 5fca4f1 Compare March 25, 2026 12:19
@rwb27 rwb27 force-pushed the validate-properties-on-assignment branch 3 times, most recently from dc056f6 to 51733e9 Compare March 30, 2026 13:21
@rwb27 rwb27 requested review from bprobert97 and julianstirling and removed request for julianstirling March 31, 2026 08:29
rwb27 added 6 commits March 31, 2026 16:55
This uses the existing validation logic in `PropertyInfo` to check assignments made in Python as well as set operations over HTTP.
I've added a module-level `FEATURE_FLAGS` object. This means validation of properties on set is disabled by default, but may be enabled if it's wanted.

The test suite checks both cases, and I've added a context manager to avoid accidental state leakage between tests.
This makes sure we can't accidentally set flags that don't exist, which ought to help downstream code.
@rwb27 rwb27 force-pushed the validate-properties-on-assignment branch from 7bd4f44 to 9cf973d Compare March 31, 2026 15:55
@rwb27 rwb27 requested a review from bprobert97 March 31, 2026 15:56
Copy link
Copy Markdown
Contributor

@bprobert97 bprobert97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've got no more comments on the code, but need to investigate the failing test

o
Test `set_temporarily` recovers from exceptions.

Thanks @bprobert97 for this test.
@rwb27 rwb27 force-pushed the validate-properties-on-assignment branch from 9cf973d to 5af5eac Compare March 31, 2026 20:45
@rwb27
Copy link
Copy Markdown
Collaborator Author

rwb27 commented Mar 31, 2026

@bprobert97 sorry about the failing test, I thought I'd checked that before requesting rereview. It's passing now (and the module hasn't changed, so I'm sure the OFM tests will pass by the morning).

@rwb27 rwb27 merged commit a6e5f78 into main Apr 1, 2026
14 checks passed
@rwb27 rwb27 deleted the validate-properties-on-assignment branch April 1, 2026 21:50
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.

2 participants