Get feature flags from config service (#1444)

* Add FeatureFlags class for params 
* Makes use of the daq-config-server to fetch data for it

setup.cfg

@@ -41,6 +41,7 @@ install_requires =
     # These dependencies may be issued as pre-release versions and should have a pin constraint
     # as by default pip-install will not upgrade to a pre-release.
     # 
+    daq-config-server@git+https://github.com/DiamondLightSource/daq-config-server.git
     ophyd == 1.9.0
     ophyd-async >= 0.3a5
     bluesky >= 1.13.0a3

src/hyperion/experiment_plans/flyscan_xray_centre_plan.py

@@ -328,7 +328,7 @@ def run_gridscan_and_move(
     with TRACER.start_span("move_to_result"):
         yield from move_x_y_z(fgs_composite.sample_motors, *xray_centre, wait=True)
 
-    if parameters.set_stub_offsets:
+    if parameters.FGS_params.set_stub_offsets:
         LOGGER.info("Recentring smargon co-ordinate system to this point.")
         yield from bps.mv(
             fgs_composite.sample_motors.stub_offsets, StubPosition.CURRENT_AS_CENTER
@@ -361,6 +361,7 @@ def flyscan_xray_centre(
 
     composite.eiger.set_detector_parameters(parameters.detector_params)
     composite.zocalo.zocalo_environment = parameters.zocalo_environment
+    parameters.features.update_self_from_server()
 
     @bpp.set_run_key_decorator(CONST.PLAN.GRIDSCAN_OUTER)
     @bpp.run_decorator(  # attach experiment metadata to the start document

src/hyperion/experiment_plans/grid_detect_then_xray_centre_plan.py

@@ -51,6 +51,7 @@
 from hyperion.external_interaction.callbacks.xray_centre.ispyb_callback import (
     ispyb_activation_wrapper,
 )
+from hyperion.external_interaction.config_server import FeatureFlags
 from hyperion.log import LOGGER
 from hyperion.parameters.gridscan import GridScanWithEdgeDetect, ThreeDGridScan
 from hyperion.utils.aperturescatterguard import (
@@ -184,7 +185,7 @@ def run_grid_detection_plan(
         parameters, grid_params_callback.get_grid_parameters()
     )
 
-    if parameters.use_panda:
+    if FeatureFlags.best_effort().use_panda_for_gridscan:
         yield from panda_flyscan_xray_centre(
             flyscan_composite,
             flyscan_xray_centre_parameters,

src/hyperion/experiment_plans/panda_flyscan_xray_centre_plan.py

@@ -240,7 +240,7 @@ def run_gridscan_and_move(
     with TRACER.start_span("move_to_result"):
         yield from move_x_y_z(fgs_composite.sample_motors, *xray_centre, wait=True)
 
-    if parameters.set_stub_offsets:
+    if parameters.panda_FGS_params.set_stub_offsets:
         LOGGER.info("Recentring smargon co-ordinate system to this point.")
         yield from bps.mv(
             fgs_composite.sample_motors.stub_offsets, StubPosition.CURRENT_AS_CENTER
@@ -272,8 +272,8 @@ def panda_flyscan_xray_centre(
     """
 
     composite.eiger.set_detector_parameters(parameters.detector_params)
-
     composite.zocalo.zocalo_environment = parameters.zocalo_environment
+    parameters.features.update_self_from_server()
 
     @bpp.set_run_key_decorator(CONST.PLAN.GRIDSCAN_OUTER)
     @bpp.run_decorator(  # attach experiment metadata to the start document

src/hyperion/external_interaction/config_server.py

@@ -0,0 +1,38 @@
+from typing import TypeVar
+
+from daq_config_server.client import ConfigServer
+from pydantic import BaseModel
+
+from hyperion.log import LOGGER
+from hyperion.parameters.constants import CONST
+
+_CONFIG_SERVER: ConfigServer | None = None
+T = TypeVar("T")
+
+
+def config_server() -> ConfigServer:
+    global _CONFIG_SERVER
+    if _CONFIG_SERVER is None:
+        _CONFIG_SERVER = ConfigServer(CONST.CONFIG_SERVER_URL, LOGGER)
+    return _CONFIG_SERVER
+
+
+class FeatureFlags(BaseModel):
+    # The default value will be used as the fallback when doing a best-effort fetch
+    # from the service
+    use_panda_for_gridscan: bool = False
+    use_gpu_for_gridscan: bool = False
+    set_stub_offsets: bool = False
+
+    @classmethod
+    def _get_flags(cls):
+        flags = config_server().best_effort_get_all_feature_flags()
+        return {f: flags[f] for f in flags if f in cls.__fields__.keys()}
+
+    @classmethod
+    def best_effort(cls):
+        return cls(**cls._get_flags())
+
+    def update_self_from_server(self):
+        for flag, value in self._get_flags().items():
+            setattr(self, flag, value)

src/hyperion/parameters/components.py

@@ -17,6 +17,7 @@
 from scanspec.core import AxesPoints
 from semver import Version
 
+from hyperion.external_interaction.config_server import FeatureFlags
 from hyperion.external_interaction.ispyb.ispyb_dataclass import (
     IspybParams,
 )
@@ -116,15 +117,16 @@ class Config:
     def __hash__(self) -> int:
         return self.json().__hash__()
 
+    features: FeatureFlags = Field(default=FeatureFlags())
     parameter_model_version: ParameterVersion
 
     @validator("parameter_model_version")
     def _validate_version(cls, version: ParameterVersion):
-        assert version >= ParameterVersion(
-            major=PARAMETER_VERSION.major
+        assert (
+            version >= ParameterVersion(major=PARAMETER_VERSION.major)
         ), f"Parameter version too old! This version of hyperion uses {PARAMETER_VERSION}"
-        assert version <= ParameterVersion(
-            major=PARAMETER_VERSION.major + 1
+        assert (
+            version <= ParameterVersion(major=PARAMETER_VERSION.major + 1)
         ), f"Parameter version too new! This version of hyperion uses {PARAMETER_VERSION}"
         return version
 

src/hyperion/parameters/constants.py

@@ -109,6 +109,7 @@ class HyperionConstants:
     TRIGGER = TriggerConstants()
     CALLBACK_0MQ_PROXY_PORTS = (5577, 5578)
     DESCRIPTORS = DocDescriptorNames()
+    CONFIG_SERVER_URL = "https://daq-config.diamond.ac.uk/api"
     GRAYLOG_PORT = 12232
     PARAMETER_SCHEMA_DIRECTORY = "src/hyperion/parameters/schemas/"
     ZOCALO_ENV = "dev_artemis" if TEST_MODE else "artemis"

src/hyperion/parameters/gridscan.py

@@ -40,7 +40,6 @@ class GridCommon(
     panda_runup_distance_mm: float = Field(
         default=CONST.HARDWARE.PANDA_FGS_RUN_UP_DEFAULT
     )
-    set_stub_offsets: bool = Field(default=False)
     use_panda: bool = Field(default=CONST.I03.USE_PANDA_FOR_GRIDSCAN)
     use_gpu: bool = Field(default=CONST.I03.USE_GPU_FOR_GRIDSCAN_ANALYSIS)
     ispyb_experiment_type: IspybExperimentType = Field(
@@ -57,6 +56,7 @@ def ispyb_params(self):
             comment=self.comment,
             sample_id=self.sample_id,
             ispyb_experiment_type=self.ispyb_experiment_type,
+            position=None,
         )
 
     @property
@@ -145,7 +145,7 @@ def FGS_params(self) -> ZebraGridScanParams:
             z1_start=self.z_start_um,
             y2_start=self.y2_start_um,
             z2_start=self.z2_start_um,
-            set_stub_offsets=False,
+            set_stub_offsets=self.features.set_stub_offsets,
             dwell_time_ms=self.exposure_time_s * 1000,
             transmission_fraction=self.transmission_frac,
         )
@@ -168,7 +168,7 @@ def panda_FGS_params(self) -> PandAGridScanParams:
             z1_start=self.z_start_um,
             y2_start=self.y2_start_um,
             z2_start=self.z2_start_um,
-            set_stub_offsets=False,
+            set_stub_offsets=self.features.set_stub_offsets,
             run_up_distance_mm=self.panda_runup_distance_mm,
             transmission_fraction=self.transmission_frac,
         )

tests/system_tests/experiment_plans/test_fgs_plan.py

@@ -303,8 +303,6 @@ def test_complete_xray_centre_plan_with_no_callbacks_falls_back_to_centre(
     params.storage_directory = "./tmp"
     params.file_name = str(uuid.uuid1())
 
-    params.set_stub_offsets = False
-
     # Currently s03 calls anything with z_steps > 1 invalid
     params.z_steps = 1
 
@@ -340,8 +338,6 @@ def test_complete_xray_centre_plan_with_callbacks_moves_to_centre(
     params.storage_directory = "./tmp"
     params.file_name = str(uuid.uuid1())
 
-    params.set_stub_offsets = False
-
     # Currently s03 calls anything with z_steps > 1 invalid
     params.z_steps = 1
 

tests/system_tests/external_interaction/test_config_service.py

@@ -0,0 +1,55 @@
+from unittest.mock import MagicMock
+from uuid import uuid4
+
+import numpy as np
+import pytest
+from daq_config_server.client import ConfigServer
+
+from hyperion.external_interaction.config_server import config_server
+
+
+@pytest.fixture
+def config_service():
+    return config_server()
+
+
+@pytest.mark.s03
+def test_get_beamline_params(config_service: ConfigServer):
+    resp = config_service.get_beamline_param("miniap_x_SMALL_APERTURE")
+    assert isinstance(resp, float)
+    assert np.isclose(resp, 2.43)
+
+
+@pytest.mark.s03
+def test_get_feature_flag(config_service: ConfigServer):
+    resp = config_service.get_feature_flag("set_stub_offsets")
+    assert isinstance(resp, bool)
+
+
+@pytest.mark.s03
+def test_get_feature_flags(config_service: ConfigServer):
+    features = config_service.best_effort_get_all_feature_flags()
+    assert len(features.keys()) == 3
+
+
+@pytest.mark.s03
+def test_nonsense_feature_flag_fails_with_normal_call(config_service: ConfigServer):
+    with pytest.raises(AssertionError):
+        _ = config_service.get_feature_flag(str(uuid4()))
+
+
+@pytest.mark.s03
+def test_best_effort_gracefully_fails_with_nonsense(config_service: ConfigServer):
+    resp = config_service.best_effort_get_feature_flag(str(uuid4()))
+    assert resp is None
+
+
+@pytest.mark.s03
+def test_best_effort_gracefully_fails_if_service_down(config_service: ConfigServer):
+    log_mock = MagicMock()
+    config_service = ConfigServer("http://not_real_address", log_mock)
+    resp = config_service.best_effort_get_feature_flag("set_stub_offsets")
+    assert resp is None
+    log_mock.error.assert_called_with(
+        "Encountered an error reading from the config service.", exc_info=True
+    )

tests/test_data/parameter_json_files/test_gridscan_param_defaults.json

@@ -26,6 +26,5 @@
     "z2_start_um": 0.0,
     "detector_distance_mm": 100.0,
     "omega_start_deg": 0.0,
-    "exposure_time_s": 0.1,
-    "set_stub_offsets": true
+    "exposure_time_s": 0.1
 }

tests/unit_tests/experiment_plans/test_flyscan_xray_centre_plan.py

@@ -420,8 +420,8 @@ def test_when_gridscan_finished_then_smargon_stub_offsets_are_set_and_dev_shm_di
         test_fgs_params: ThreeDGridScan,
         fake_fgs_composite: FlyScanXRayCentreComposite,
     ):
-        test_fgs_params.set_stub_offsets = True
         RE, (nexus_cb, ispyb_cb) = RE_with_subs
+        test_fgs_params.features.set_stub_offsets = True
 
         fake_fgs_composite.eiger.odin.fan.dev_shm_enable.sim_put(1)  # type: ignore
 
@@ -646,7 +646,7 @@ def test_given_setting_stub_offsets_disabled_then_stub_offsets_not_set(
         fake_fgs_composite.aperture_scatterguard.set = MagicMock(
             return_value=done_status
         )
-        test_fgs_params.FGS_params.set_stub_offsets = False
+        test_fgs_params.features.set_stub_offsets = False
 
         def wrapped_gridscan_and_move():
             run_generic_ispyb_handler_setup(ispyb_cb, test_fgs_params)

tests/unit_tests/experiment_plans/test_panda_flyscan_xray_centre_plan.py

@@ -386,7 +386,7 @@ def test_when_gridscan_finished_then_smargon_stub_offsets_are_set_and_dev_shm_di
         test_panda_fgs_params: ThreeDGridScan,
         fake_fgs_composite: FlyScanXRayCentreComposite,
     ):
-        test_panda_fgs_params.set_stub_offsets = True
+        test_panda_fgs_params.features.set_stub_offsets = True
 
         fake_fgs_composite.eiger.odin.fan.dev_shm_enable.sim_put(1)  # type: ignore
 
@@ -454,9 +454,7 @@ def wrapped_run_gridscan_and_move():
                 wrapped_run_gridscan_and_move(), test_panda_fgs_params
             )
         )
-        app_to_comment: MagicMock = mock_subscriptions[
-            1
-        ].ispyb.append_to_comment  # type:ignore
+        app_to_comment: MagicMock = mock_subscriptions[1].ispyb.append_to_comment  # type:ignore
         app_to_comment.assert_called()
         call = app_to_comment.call_args_list[0]
         assert "Crystal 1: Strength 999999" in call.args[1]
@@ -499,9 +497,7 @@ def wrapped_run_gridscan_and_move():
                 wrapped_run_gridscan_and_move(), test_panda_fgs_params
             )
         )
-        app_to_comment: MagicMock = mock_subscriptions[
-            1
-        ].ispyb.append_to_comment  # type:ignore
+        app_to_comment: MagicMock = mock_subscriptions[1].ispyb.append_to_comment  # type:ignore
         app_to_comment.assert_called()
         call = app_to_comment.call_args_list[0]
         assert "Zocalo found no crystals in this gridscan" in call.args[1]
@@ -628,7 +624,7 @@ def test_given_setting_stub_offsets_disabled_then_stub_offsets_not_set(
         fake_fgs_composite.aperture_scatterguard.set = MagicMock(
             return_value=done_status
         )
-        test_panda_fgs_params.set_stub_offsets = False
+        test_panda_fgs_params.features.set_stub_offsets = False
 
         RE.subscribe(VerbosePlanExecutionLoggingCallback())
         RE.subscribe(ispyb_cb)

tests/unit_tests/experiment_plans/test_pin_centre_then_xray_centre_plan.py

@@ -93,7 +93,6 @@ def test_when_pin_centre_xray_centre_called_then_detector_positioned(
         "x_step_size_um": 0.1,
         "y_step_size_um": 0.1,
         "z_step_size_um": 0.1,
-        "set_stub_offsets": False,
     }
 
     sim_run_engine.add_handler_for_callback_subscribes()