-
Notifications
You must be signed in to change notification settings - Fork 12
Migrate BeamlineParameters to use config server #1773
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
b4dbd8e
bf08ced
91fc322
386b521
acd5805
28b7823
84f0836
afd4fbc
2d92790
e3f6fad
3dbb881
9517b6a
bc858b2
9529dc1
4aac691
cba50ab
eaa20a2
0d29dbe
feae3b6
d4fda7b
a947f60
d5bd077
4b4a3f1
357a6c9
286a17c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,75 +1,26 @@ | ||
| import ast | ||
| from typing import Any, cast | ||
| from typing import Any | ||
|
|
||
| from dodal.log import LOGGER | ||
| from dodal.utils import get_beamline_name | ||
|
|
||
| BEAMLINE_PARAMETER_KEYWORDS = ["FB", "FULL", "deadtime"] | ||
| from dodal.common.beamlines.config_client import get_config_client | ||
|
|
||
| BEAMLINE_PARAMETER_PATHS = { | ||
| "i03": "/dls_sw/i03/software/daq_configuration/domain/beamlineParameters", | ||
| "i04": "/dls_sw/i04/software/daq_configuration/domain/beamlineParameters", | ||
| } | ||
|
|
||
|
|
||
| class GDABeamlineParameters: | ||
| params: dict[str, Any] | ||
|
|
||
| def __init__(self, params: dict[str, Any]): | ||
| self.params = params | ||
|
|
||
| def __repr__(self) -> str: | ||
| return repr(self.params) | ||
|
|
||
| def __getitem__(self, item: str): | ||
| return self.params[item] | ||
|
|
||
| @classmethod | ||
| def from_lines(cls, file_name: str, config_lines: list[str]): | ||
| config_lines_nocomments = [line.split("#", 1)[0] for line in config_lines] | ||
| config_lines_sep_key_and_value = [ | ||
| # XXX removes all whitespace instead of just trim | ||
| line.translate(str.maketrans("", "", " \n\t\r")).split("=") | ||
| for line in config_lines_nocomments | ||
| ] | ||
| config_pairs: list[tuple[str, Any]] = [ | ||
| cast(tuple[str, Any], param) | ||
| for param in config_lines_sep_key_and_value | ||
| if len(param) == 2 | ||
| ] | ||
| for i, (param, value) in enumerate(config_pairs): | ||
| try: | ||
| # BEAMLINE_PARAMETER_KEYWORDS effectively raw string but whitespace removed | ||
| if value not in BEAMLINE_PARAMETER_KEYWORDS: | ||
| config_pairs[i] = ( | ||
| param, | ||
| cls.parse_value(value), | ||
| ) | ||
| except Exception as e: | ||
| LOGGER.warning(f"Unable to parse {file_name} line {i}: {e}") | ||
|
|
||
| return cls(params=dict(config_pairs)) | ||
|
|
||
| @classmethod | ||
| def from_file(cls, path: str): | ||
| with open(path) as f: | ||
| config_lines = f.readlines() | ||
| return cls.from_lines(path, config_lines) | ||
|
|
||
| @classmethod | ||
| def parse_value(cls, value: str): | ||
| return ast.literal_eval(value.replace("Yes", "True").replace("No", "False")) | ||
| def get_beamline_parameters(beamline: str) -> dict[str, Any]: | ||
| """Loads the beamline parameters for a specified beamline from the config server. | ||
|
|
||
| Args: | ||
| beamline (str): The beamline for which beamline parameters will be retrieved. | ||
|
|
||
| def get_beamline_parameters(beamline_param_path: str | None = None): | ||
| """Loads the beamline parameters from the specified path, or according to the | ||
| environment variable if none is given. | ||
| Returns: | ||
| dict[str, Any]: Dict of beamline parameters. | ||
| """ | ||
| if not beamline_param_path: | ||
| beamline_name = get_beamline_name("i03") | ||
| beamline_param_path = BEAMLINE_PARAMETER_PATHS.get(beamline_name) | ||
| if beamline_param_path is None: | ||
| raise KeyError( | ||
| "No beamline parameter path found, maybe 'BEAMLINE' environment variable is not set!" | ||
| ) | ||
| return GDABeamlineParameters.from_file(beamline_param_path) | ||
| beamline_param_path = BEAMLINE_PARAMETER_PATHS.get(beamline) | ||
| if beamline_param_path is None: | ||
| raise KeyError( | ||
| "No beamline parameter path found, maybe 'BEAMLINE' environment variable is not set!" | ||
| ) | ||
| config_client = get_config_client(beamline) | ||
rtuck99 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return config_client.get_file_contents(beamline_param_path, dict) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| from functools import cache | ||
|
|
||
| from daq_config_server.client import ConfigServer | ||
|
|
||
| BEAMLINE_CONFIG_SERVER_ENDPOINTS = { | ||
| "i03": "https://i03-daq-config.diamond.ac.uk", | ||
| "i04": "https://daq-config.diamond.ac.uk", | ||
| } | ||
|
|
||
|
|
||
| @cache | ||
| def get_config_client(beamline: str) -> ConfigServer: | ||
| url = BEAMLINE_CONFIG_SERVER_ENDPOINTS.get( | ||
| beamline, "https://daq-config.diamond.ac.uk" | ||
| ) | ||
| return ConfigServer(url=url) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |
| ) | ||
| from dodal.devices.synchrotron import Synchrotron, SynchrotronMode | ||
| from dodal.log import LOGGER | ||
| from dodal.utils import get_beamline_name | ||
|
|
||
| ALLOWED_MODES = [SynchrotronMode.USER, SynchrotronMode.SPECIAL] | ||
| DECAY_MODE_COUNTDOWN = -1 # Value of the start_countdown PV when in decay mode | ||
|
|
@@ -133,5 +134,5 @@ def check_topup_and_wait_if_necessary( | |
|
|
||
|
|
||
| def _load_topup_configuration_from_properties_file() -> dict[str, Any]: | ||
| params = get_beamline_parameters() | ||
| return params.params | ||
| params = get_beamline_parameters(get_beamline_name("i03")) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't really like this practice that we have in the code of inserting arbitrary default beamline names into random bits of code. By removing the default from I think it would be better if we never assumed it anywhere in production code and instead took it from the BEAMLINE environment variable in a single function which we can patch out for unit tests.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So basically remove the 'default' argument from
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've had a quick go at doing this and it's looking like a big change, and I'm not 100% sure of the consequences, so I think we should pull it out into a new issue. For now, I'll make the |
||
| return params | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| import json | ||
| from pathlib import Path | ||
| from typing import TypeVar | ||
| from unittest.mock import patch | ||
|
|
||
| import pytest | ||
| from daq_config_server.models import ConfigModel | ||
|
|
||
| T = TypeVar("T", str, dict, ConfigModel) | ||
|
|
||
|
|
||
| def fake_config_server_get_file_contents( | ||
| filepath: str | Path, | ||
| desired_return_type: type[T] = str, | ||
| reset_cached_result: bool = True, | ||
| ) -> T: | ||
| filepath = Path(filepath) | ||
| # Minimal logic required for unit tests | ||
| with filepath.open("r") as f: | ||
| contents = f.read() | ||
| if desired_return_type is str: | ||
| return contents # type: ignore | ||
| elif desired_return_type is dict: | ||
| return json.loads(contents) | ||
| elif issubclass(desired_return_type, ConfigModel): | ||
| return desired_return_type.model_validate(json.loads(contents)) | ||
| raise ValueError("Invalid return type requested") | ||
|
|
||
|
|
||
| @pytest.fixture(autouse=True) | ||
| def mock_config_server(): | ||
| # Don't actually talk to central service during unit tests, and reset caches between test | ||
|
|
||
| with patch( | ||
| "daq_config_server.client.ConfigServer.get_file_contents", | ||
| side_effect=fake_config_server_get_file_contents, | ||
| ): | ||
| yield |
Uh oh!
There was an error while loading. Please reload this page.