Skip to content

Commit 5fca4f1

Browse files
committed
Move validation behind a feature flag.
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.
1 parent 6da99f0 commit 5fca4f1

File tree

5 files changed

+81
-6
lines changed

5 files changed

+81
-6
lines changed

src/labthings_fastapi/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
from .properties import property, setting, DataProperty, DataSetting
2626
from .actions import action
2727
from .endpoints import endpoint
28+
from .feature_flags import FEATURE_FLAGS
2829
from . import outputs
2930
from .outputs import blob
3031
from .server import ThingServer, cli
@@ -52,6 +53,7 @@
5253
"action",
5354
"thing_slot",
5455
"endpoint",
56+
"FEATURE_FLAGS",
5557
"outputs",
5658
"blob",
5759
"ThingServer",
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
"""Control of optional features in LabThings.
2+
3+
This module provides a way to globally enable or disable certain features of LabThings.
4+
When a new, non-backwards-compatible feature is added, it will usually be disabled by
5+
default. This module provides an object `.FEATURE_FLAGS` that allows control over
6+
optional features.
7+
8+
The default values of `.FEATURE_FLAGS` may change with new LabThings releases. The usual
9+
sequence of events would be that a new feature is added (disabled by default), then the
10+
feature flag is enabled by default in a subsequent release. If the intention is that the
11+
feature will become non-optional, disabling the feature will raise a
12+
`DeprecationWarning` for at least one release cycle before it is removed.
13+
"""
14+
15+
from collections.abc import Iterator
16+
from contextlib import contextmanager
17+
from dataclasses import dataclass
18+
from typing import Any
19+
20+
21+
@dataclass
22+
class LabthingsFeatureFlags:
23+
"""Control over optional features of LabThings."""
24+
25+
validate_properties_on_set: bool = False
26+
"""Whether validation logic runs when properties are set in Python."""
27+
28+
@contextmanager
29+
def set_temporarily(
30+
self,
31+
**kwargs: Any,
32+
) -> Iterator[None]:
33+
r"""Temporarily set feature flags in a context manager.
34+
35+
This function may be used in a `with:` block to set feature flags and
36+
then reset them afterwards. This is primarily useful for testing.
37+
38+
:param \**kwargs: the feature flags to set for the duration of the ``with:``
39+
block. The argument names must match attributes of this object.
40+
41+
.. code-block: python
42+
43+
with FEATURE_FLAGS.set_temporarily(validate_properties_on_set=True):
44+
my_thing.positive_int = -10 # Raises an exception
45+
46+
"""
47+
values_before = {k: getattr(self, k) for k in kwargs.keys()}
48+
try:
49+
for k, v in kwargs.items():
50+
setattr(self, k, v)
51+
yield
52+
finally:
53+
for k, v in values_before.items():
54+
setattr(self, k, v)
55+
56+
57+
FEATURE_FLAGS = LabthingsFeatureFlags()

src/labthings_fastapi/properties.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ class attribute. Documentation is in strings immediately following the
6464
from fastapi import Body, FastAPI
6565
from pydantic import BaseModel, ConfigDict, RootModel, ValidationError, create_model
6666

67+
from .feature_flags import FEATURE_FLAGS
6768
from .thing_description import type_to_dataschema
6869
from .thing_description._model import (
6970
DataSchema,
@@ -609,7 +610,10 @@ def __set__(
609610
:param emit_changed_event: whether to emit a changed event.
610611
"""
611612
property_info = self.descriptor_info(obj)
612-
obj.__dict__[self.name] = property_info.validate(value)
613+
if FEATURE_FLAGS.validate_properties_on_set:
614+
obj.__dict__[self.name] = property_info.validate(value)
615+
else:
616+
obj.__dict__[self.name] = value
613617
if emit_changed_event:
614618
self.emit_changed_event(obj, value)
615619

@@ -812,7 +816,8 @@ def instance_get(self, obj: Owner) -> Value:
812816
def __set__(self, obj: Owner, value: Value) -> None:
813817
"""Set the value of the property.
814818
815-
This will validate the value against the property's model, and an error
819+
If property validation is enabled by `.FEATURE_FLAGS.validate_properties_on_set`
820+
this will validate the value against the property's model, and an error
816821
will be raised if the value is not valid.
817822
818823
:param obj: the `.Thing` on which the attribute is accessed.
@@ -823,7 +828,8 @@ def __set__(self, obj: Owner, value: Value) -> None:
823828
if self.fset is None:
824829
raise ReadOnlyPropertyError(f"Property {self.name} of {obj} has no setter.")
825830
property_info = self.descriptor_info(obj)
826-
value = property_info.validate(value)
831+
if FEATURE_FLAGS.validate_properties_on_set:
832+
value = property_info.validate(value)
827833

828834
self.fset(obj, value)
829835

tests/test_properties.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
ServerNotRunningError,
1515
UnsupportedConstraintError,
1616
)
17+
from labthings_fastapi.feature_flags import FEATURE_FLAGS
1718
from labthings_fastapi.properties import BaseProperty, PropertyInfo
1819
from labthings_fastapi.testing import create_thing_without_server
1920
from .temp_client import poll_task
@@ -409,13 +410,20 @@ def test_constrained_properties(prop_info, mocker):
409410
prop.__set__(mock_thing, valid)
410411
validated = prop.__get__(mock_thing)
411412
assert validated == valid or validated is valid # `is` for NaN
413+
412414
for invalid in prop_info.invalid_values:
413415
with pytest.raises(ValidationError):
414416
_ = m(root=invalid)
415417
with pytest.raises(ValidationError):
416418
descriptorinfo.validate(invalid)
417-
with pytest.raises(ValidationError):
419+
# We check that validation on __set__ only applies if it's enabled.
420+
with FEATURE_FLAGS.set_temporarily(validate_properties_on_set=True):
421+
with pytest.raises(ValidationError):
422+
prop.__set__(mock_thing, invalid)
423+
with FEATURE_FLAGS.set_temporarily(validate_properties_on_set=False):
418424
prop.__set__(mock_thing, invalid)
425+
not_validated = prop.__get__(mock_thing)
426+
assert not_validated == invalid or not_validated is invalid
419427

420428

421429
def convert_inf_nan(value):

tests/test_property.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import pydantic
1717
import pytest
1818
from labthings_fastapi import properties
19+
from labthings_fastapi.feature_flags import FEATURE_FLAGS
1920
from labthings_fastapi.properties import (
2021
BaseProperty,
2122
DataProperty,
@@ -365,8 +366,9 @@ class BadIntModel(pydantic.BaseModel):
365366
# Check that a broken `_model` raises the right error
366367
# See above for where we manually set badprop._model to something that's
367368
# not a rootmodel.
368-
with pytest.raises(TypeError):
369-
example.badprop = 3 # Validation will fail here because of the bad model.
369+
with FEATURE_FLAGS.set_temporarily(validate_properties_on_set=True):
370+
with pytest.raises(TypeError):
371+
example.badprop = 3 # Validation will fail here because of the bad model.
370372
with pytest.raises(TypeError):
371373
_ = example.properties["badprop"].model_instance
372374
with pytest.raises(TypeError):

0 commit comments

Comments
 (0)