From fee5c0623fc45bc660a10d93b1d14cbfa3ea688c Mon Sep 17 00:00:00 2001 From: Joseph Ware <53935796+DiamondJoseph@users.noreply.github.com> Date: Mon, 17 Nov 2025 13:01:41 +0000 Subject: [PATCH 1/3] fix: Use standard Logger construction --- src/blueapi/cli/cli.py | 4 +++- src/blueapi/cli/scratch.py | 12 +++++++----- src/blueapi/client/numtracker.py | 4 +++- src/blueapi/worker/task_worker.py | 2 +- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 957a9cb27..1b7ee58e5 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -42,6 +42,8 @@ from .scratch import setup_scratch from .updates import CliEventRenderer +LOGGER = logging.getLogger(__name__) + @click.group( invoke_without_command=True, context_settings={"auto_envvar_prefix": "BLUEAPI"} @@ -493,7 +495,7 @@ def logout(obj: dict) -> None: except FileNotFoundError: print("Logged out") except ValueError as e: - logging.debug("Invalid login token: %s", e) + LOGGER.debug("Invalid login token: %s", e) raise ClickException( "Login token is not valid - remove before trying again" ) from e diff --git a/src/blueapi/cli/scratch.py b/src/blueapi/cli/scratch.py index 40b91016a..841713441 100644 --- a/src/blueapi/cli/scratch.py +++ b/src/blueapi/cli/scratch.py @@ -15,6 +15,8 @@ _DEFAULT_INSTALL_TIMEOUT: float = 300.0 +LOGGER = logging.getLogger(__name__) + def setup_scratch( config: ScratchConfig, install_timeout: float = _DEFAULT_INSTALL_TIMEOUT @@ -30,7 +32,7 @@ def setup_scratch( _validate_root_directory(config.root, config.required_gid) - logging.info(f"Setting up scratch area: {config.root}") + LOGGER.info(f"Setting up scratch area: {config.root}") """ fail early """ for repo in config.repositories: @@ -64,12 +66,12 @@ def ensure_repo(remote_url: str, local_directory: Path) -> None: os.umask(stat.S_IWOTH) if not local_directory.exists(): - logging.info(f"Cloning {remote_url}") + LOGGER.info(f"Cloning {remote_url}") Repo.clone_from(remote_url, local_directory) - logging.info(f"Cloned {remote_url} -> {local_directory}") + LOGGER.info(f"Cloned {remote_url} -> {local_directory}") elif local_directory.is_dir(): Repo(local_directory) - logging.info(f"Found {local_directory}") + LOGGER.info(f"Found {local_directory}") else: raise KeyError( f"Unable to open {local_directory} as a git repository because it is a file" @@ -90,7 +92,7 @@ def scratch_install(path: Path, timeout: float = _DEFAULT_INSTALL_TIMEOUT) -> No _validate_directory(path) - logging.info(f"Installing {path}") + LOGGER.info(f"Installing {path}") process = Popen( [ "python", diff --git a/src/blueapi/client/numtracker.py b/src/blueapi/client/numtracker.py index 3ec7a9ab8..9fb83ec95 100644 --- a/src/blueapi/client/numtracker.py +++ b/src/blueapi/client/numtracker.py @@ -8,6 +8,8 @@ from blueapi.utils import BlueapiBaseModel +LOGGER = logging.getLogger(__name__) + class DirectoryPath(BlueapiBaseModel): """ @@ -105,5 +107,5 @@ def create_scan( raise RuntimeError(f"Numtracker error: {json['errors']}") new_collection = NumtrackerScanMutationResponse.model_validate(json["data"]) - logging.debug("New NumtrackerNewScan: %s", new_collection) + LOGGER.debug("New NumtrackerNewScan: %s", new_collection) return new_collection diff --git a/src/blueapi/worker/task_worker.py b/src/blueapi/worker/task_worker.py index 06055cc93..62435d261 100644 --- a/src/blueapi/worker/task_worker.py +++ b/src/blueapi/worker/task_worker.py @@ -273,7 +273,7 @@ def submit_task(self, task: Task) -> str: request_id = get_baggage("correlation_id") # If request id is not a string, we do not pass it into a TrackableTask if not isinstance(request_id, str): - logging.warning(f"Invalid correlation id detected: {request_id}") + LOGGER.warning(f"Invalid correlation id detected: {request_id}") request_id = None trackable_task = TrackableTask( task_id=task_id, From a8f2a21eb48e2ac71bbebff9ae2258fce98ac057 Mon Sep 17 00:00:00 2001 From: Joseph Ware <53935796+DiamondJoseph@users.noreply.github.com> Date: Mon, 17 Nov 2025 15:24:44 +0000 Subject: [PATCH 2/3] feat: Allow checking out scratch area on target revisions --- helm/blueapi/config_schema.json | 31 +++++++++++++++++++++++++ helm/blueapi/values.schema.json | 29 ++++++++++++++++++++++++ src/blueapi/cli/scratch.py | 40 ++++++++++++++++++++++++++++----- src/blueapi/config.py | 19 ++++++++++++++++ 4 files changed, 114 insertions(+), 5 deletions(-) diff --git a/helm/blueapi/config_schema.json b/helm/blueapi/config_schema.json index 03535cf48..416bf67d0 100644 --- a/helm/blueapi/config_schema.json +++ b/helm/blueapi/config_schema.json @@ -242,6 +242,31 @@ "title": "RestConfig", "type": "object" }, + "RevisionConfig": { + "additionalProperties": false, + "properties": { + "branch": { + "default": "main", + "description": "Branch name to check out OR create if checking out a tag", + "title": "Branch", + "type": "string" + }, + "tag": { + "default": null, + "description": "Tag to check out, if named branch does not exist", + "title": "Tag", + "type": "string" + }, + "pull": { + "default": false, + "description": "Whether to pull remote changes after checking out a branch", + "title": "Pull", + "type": "boolean" + } + }, + "title": "RevisionConfig", + "type": "object" + }, "ScratchConfig": { "additionalProperties": false, "properties": { @@ -291,6 +316,12 @@ "description": "URL to clone from", "title": "Remote Url", "type": "string" + }, + "target_revision": { + "$ref": "#/$defs/RevisionConfig", + "default": null, + "description": "Target revision to check out for the repository", + "title": "Target Revision" } }, "title": "ScratchRepository", diff --git a/helm/blueapi/values.schema.json b/helm/blueapi/values.schema.json index b1baea653..cdafe06cb 100644 --- a/helm/blueapi/values.schema.json +++ b/helm/blueapi/values.schema.json @@ -726,6 +726,30 @@ }, "additionalProperties": false }, + "RevisionConfig": { + "title": "RevisionConfig", + "type": "object", + "properties": { + "branch": { + "title": "Branch", + "description": "Branch name to check out OR create if checking out a tag", + "default": "main", + "type": "string" + }, + "pull": { + "title": "Pull", + "description": "Whether to pull remote changes after checking out a branch", + "default": false, + "type": "boolean" + }, + "tag": { + "title": "Tag", + "description": "Tag to check out, if named branch does not exist", + "type": "string" + } + }, + "additionalProperties": false + }, "ScratchConfig": { "title": "ScratchConfig", "type": "object", @@ -774,6 +798,11 @@ "description": "URL to clone from", "default": "https://github.com/example/example.git", "type": "string" + }, + "target_revision": { + "title": "Target Revision", + "description": "Target revision to check out for the repository", + "$ref": "#/$defs/RevisionConfig" } }, "additionalProperties": false diff --git a/src/blueapi/cli/scratch.py b/src/blueapi/cli/scratch.py index 841713441..3249a3fe2 100644 --- a/src/blueapi/cli/scratch.py +++ b/src/blueapi/cli/scratch.py @@ -9,7 +9,7 @@ from git import Repo from tomlkit import parse -from blueapi.config import FORBIDDEN_OWN_REMOTE_URL, ScratchConfig +from blueapi.config import FORBIDDEN_OWN_REMOTE_URL, RevisionConfig, ScratchConfig from blueapi.service.model import PackageInfo, PythonEnvironmentResponse, SourceInfo from blueapi.utils import get_owner_gid, is_sgid_set @@ -48,11 +48,13 @@ def setup_scratch( ) for repo in config.repositories: local_directory = config.root / repo.name - ensure_repo(repo.remote_url, local_directory) + repository = ensure_repo(repo.remote_url, local_directory) + if repo.target_revision: + checkout_target(repository, repo.target_revision) scratch_install(local_directory, timeout=install_timeout) -def ensure_repo(remote_url: str, local_directory: Path) -> None: +def ensure_repo(remote_url: str, local_directory: Path) -> Repo: """ Ensure that a repository is checked out for use in the scratch area. Clone it if it isn't. @@ -67,17 +69,45 @@ def ensure_repo(remote_url: str, local_directory: Path) -> None: if not local_directory.exists(): LOGGER.info(f"Cloning {remote_url}") - Repo.clone_from(remote_url, local_directory) + repo = Repo.clone_from(remote_url, local_directory) LOGGER.info(f"Cloned {remote_url} -> {local_directory}") + return repo elif local_directory.is_dir(): - Repo(local_directory) + repo = Repo(local_directory) LOGGER.info(f"Found {local_directory}") + return repo else: raise KeyError( f"Unable to open {local_directory} as a git repository because it is a file" ) +def checkout_target(repo: Repo, target_revision: RevisionConfig) -> None: + LOGGER.info(f"{repo.working_dir}: fetching") + repo.remote().fetch() + LOGGER.info( + f"{repo.working_dir}: looking for existing branch {target_revision.branch}" + ) + for branch in repo.branches: + if branch.name == target_revision.branch: + LOGGER.info( + f"{repo.working_dir}: checking out existing branch {branch.name}" + ) + repo.head.reference = branch + else: + LOGGER.info(f"{repo.working_dir}: no existing branch {target_revision.branch}") + if target_revision.tag_name: + LOGGER.info( + f"{repo.working_dir}: checking out tag {target_revision.tag_name}" + ) + repo.head.reference = repo.tag(target_revision.tag_name) + LOGGER.info(f"{repo.working_dir}: creating branch {target_revision.branch}") + repo.create_head(target_revision.branch) + if target_revision.pull: + LOGGER.info(f"{repo.working_dir}: pulling") + repo.remote().pull() + + def scratch_install(path: Path, timeout: float = _DEFAULT_INSTALL_TIMEOUT) -> None: """ Install a scratch package. Make blueapi aware of a repository checked out in diff --git a/src/blueapi/config.py b/src/blueapi/config.py index 82c8f5e3e..8acd7efc9 100644 --- a/src/blueapi/config.py +++ b/src/blueapi/config.py @@ -21,6 +21,7 @@ ValidationError, field_validator, ) +from pydantic.json_schema import SkipJsonSchema from blueapi.utils import BlueapiBaseModel, InvalidConfigError @@ -131,6 +132,20 @@ class RestConfig(BlueapiBaseModel): cors: CORSConfig | None = None +class RevisionConfig(BlueapiBaseModel): + branch: str = Field( + description="Branch name to check out OR create if checking out a tag", + default="main", + ) + tag: str | SkipJsonSchema[None] = Field( + description="Tag to check out, if named branch does not exist", default=None + ) + pull: bool = Field( + description="Whether to pull remote changes after checking out a branch", + default=False, + ) + + class ScratchRepository(BlueapiBaseModel): name: str = Field( description="Unique name for this repository in the scratch directory", @@ -140,6 +155,10 @@ class ScratchRepository(BlueapiBaseModel): description="URL to clone from", default="https://github.com/example/example.git", ) + target_revision: RevisionConfig | SkipJsonSchema[None] = Field( + description="Target revision to check out for the repository", + default=None, + ) @field_validator("remote_url") @classmethod From 9e5e1b49b590f3b71988907ce1d01a94f236d530 Mon Sep 17 00:00:00 2001 From: Joseph Ware <53935796+DiamondJoseph@users.noreply.github.com> Date: Tue, 18 Nov 2025 14:37:40 +0000 Subject: [PATCH 3/3] refactor: Simplify handling --- helm/blueapi/config_schema.json | 46 ++++++++++++++++-------- helm/blueapi/values.schema.json | 42 +++++++++++++++------- src/blueapi/cli/scratch.py | 62 +++++++++++++++++++-------------- src/blueapi/config.py | 23 +++++++----- 4 files changed, 112 insertions(+), 61 deletions(-) diff --git a/helm/blueapi/config_schema.json b/helm/blueapi/config_schema.json index 416bf67d0..eb20eb9d3 100644 --- a/helm/blueapi/config_schema.json +++ b/helm/blueapi/config_schema.json @@ -64,6 +64,21 @@ "title": "CORSConfig", "type": "object" }, + "DependencyReference": { + "additionalProperties": false, + "properties": { + "dependency": { + "description": "Python module name to try and check out the version of- e.g. `dls-dodal` for dodal, as that is the python module name", + "title": "Dependency", + "type": "string" + } + }, + "required": [ + "dependency" + ], + "title": "DependencyReference", + "type": "object" + }, "EnvironmentConfig": { "additionalProperties": false, "description": "Config for the RunEngine environment", @@ -245,25 +260,28 @@ "RevisionConfig": { "additionalProperties": false, "properties": { - "branch": { - "default": "main", - "description": "Branch name to check out OR create if checking out a tag", - "title": "Branch", - "type": "string" + "reference": { + "anyOf": [ + { + "type": "string" + }, + { + "$ref": "#/$defs/DependencyReference" + } + ], + "description": "Reference to check out- either a git reference (tag, branch,commit) or a reference to a python dependency", + "title": "Reference" }, - "tag": { + "branch": { "default": null, - "description": "Tag to check out, if named branch does not exist", - "title": "Tag", + "description": "Branch name to create if checking out a reference not on a branch, to avoid leaving head detached", + "title": "Branch", "type": "string" - }, - "pull": { - "default": false, - "description": "Whether to pull remote changes after checking out a branch", - "title": "Pull", - "type": "boolean" } }, + "required": [ + "reference" + ], "title": "RevisionConfig", "type": "object" }, diff --git a/helm/blueapi/values.schema.json b/helm/blueapi/values.schema.json index cdafe06cb..4da767344 100644 --- a/helm/blueapi/values.schema.json +++ b/helm/blueapi/values.schema.json @@ -553,6 +553,21 @@ }, "additionalProperties": false }, + "DependencyReference": { + "title": "DependencyReference", + "type": "object", + "required": [ + "dependency" + ], + "properties": { + "dependency": { + "title": "Dependency", + "description": "Python module name to try and check out the version of- e.g. `dls-dodal` for dodal, as that is the python module name", + "type": "string" + } + }, + "additionalProperties": false + }, "EnvironmentConfig": { "title": "EnvironmentConfig", "description": "Config for the RunEngine environment", @@ -729,23 +744,26 @@ "RevisionConfig": { "title": "RevisionConfig", "type": "object", + "required": [ + "reference" + ], "properties": { "branch": { "title": "Branch", - "description": "Branch name to check out OR create if checking out a tag", - "default": "main", + "description": "Branch name to create if checking out a reference not on a branch, to avoid leaving head detached", "type": "string" }, - "pull": { - "title": "Pull", - "description": "Whether to pull remote changes after checking out a branch", - "default": false, - "type": "boolean" - }, - "tag": { - "title": "Tag", - "description": "Tag to check out, if named branch does not exist", - "type": "string" + "reference": { + "title": "Reference", + "description": "Reference to check out- either a git reference (tag, branch,commit) or a reference to a python dependency", + "anyOf": [ + { + "type": "string" + }, + { + "$ref": "#/$defs/DependencyReference" + } + ] } }, "additionalProperties": false diff --git a/src/blueapi/cli/scratch.py b/src/blueapi/cli/scratch.py index 3249a3fe2..f13f79f13 100644 --- a/src/blueapi/cli/scratch.py +++ b/src/blueapi/cli/scratch.py @@ -6,10 +6,14 @@ from pathlib import Path from subprocess import Popen -from git import Repo +from git import HEAD, Head, Repo from tomlkit import parse -from blueapi.config import FORBIDDEN_OWN_REMOTE_URL, RevisionConfig, ScratchConfig +from blueapi.config import ( + FORBIDDEN_OWN_REMOTE_URL, + DependencyReference, + ScratchConfig, +) from blueapi.service.model import PackageInfo, PythonEnvironmentResponse, SourceInfo from blueapi.utils import get_owner_gid, is_sgid_set @@ -50,7 +54,9 @@ def setup_scratch( local_directory = config.root / repo.name repository = ensure_repo(repo.remote_url, local_directory) if repo.target_revision: - checkout_target(repository, repo.target_revision) + checkout_target( + repository, repo.target_revision.reference, repo.target_revision.branch + ) scratch_install(local_directory, timeout=install_timeout) @@ -74,7 +80,8 @@ def ensure_repo(remote_url: str, local_directory: Path) -> Repo: return repo elif local_directory.is_dir(): repo = Repo(local_directory) - LOGGER.info(f"Found {local_directory}") + LOGGER.info(f"Found {local_directory} - fetching") + repo.remote().fetch() return repo else: raise KeyError( @@ -82,30 +89,33 @@ def ensure_repo(remote_url: str, local_directory: Path) -> Repo: ) -def checkout_target(repo: Repo, target_revision: RevisionConfig) -> None: - LOGGER.info(f"{repo.working_dir}: fetching") - repo.remote().fetch() - LOGGER.info( - f"{repo.working_dir}: looking for existing branch {target_revision.branch}" - ) - for branch in repo.branches: - if branch.name == target_revision.branch: - LOGGER.info( - f"{repo.working_dir}: checking out existing branch {branch.name}" - ) - repo.head.reference = branch - else: - LOGGER.info(f"{repo.working_dir}: no existing branch {target_revision.branch}") - if target_revision.tag_name: +def checkout_target( + repo: Repo, target_revision: str | DependencyReference, branch_name: str | None +) -> Head | HEAD: + if isinstance(target_revision, DependencyReference): + LOGGER.info( + f"{repo.working_dir}: attempting to check out version" + " matching {target_revision.dependency}" + ) + version = importlib.metadata.version(target_revision.dependency) + try: + return checkout_target(repo, version, branch_name) + except ValueError: LOGGER.info( - f"{repo.working_dir}: checking out tag {target_revision.tag_name}" + f"{repo.working_dir}: no ref maching version {version}," + " attempting v{version}" ) - repo.head.reference = repo.tag(target_revision.tag_name) - LOGGER.info(f"{repo.working_dir}: creating branch {target_revision.branch}") - repo.create_head(target_revision.branch) - if target_revision.pull: - LOGGER.info(f"{repo.working_dir}: pulling") - repo.remote().pull() + return checkout_target(repo, "v" + version, branch_name) + LOGGER.info(f"{repo.working_dir}: attempting to check out {target_revision}") + for ref in repo.refs: + if ref.name == target_revision: + repo.head.reference = ref + if repo.head.is_detached and branch_name: + repo.create_head(branch_name) + return repo.head + raise ValueError( + f"Unable to find target revision {target_revision} for repo {repo.working_dir}" + ) def scratch_install(path: Path, timeout: float = _DEFAULT_INSTALL_TIMEOUT) -> None: diff --git a/src/blueapi/config.py b/src/blueapi/config.py index 8acd7efc9..4c9786ed9 100644 --- a/src/blueapi/config.py +++ b/src/blueapi/config.py @@ -132,17 +132,22 @@ class RestConfig(BlueapiBaseModel): cors: CORSConfig | None = None -class RevisionConfig(BlueapiBaseModel): - branch: str = Field( - description="Branch name to check out OR create if checking out a tag", - default="main", +class DependencyReference(BlueapiBaseModel): + dependency: str = Field( + description="Python module name to try and check out the version of- e.g. " + "`dls-dodal` for dodal, as that is the python module name" ) - tag: str | SkipJsonSchema[None] = Field( - description="Tag to check out, if named branch does not exist", default=None + + +class RevisionConfig(BlueapiBaseModel): + reference: str | DependencyReference = Field( + description="Reference to check out- either a git reference (tag, branch," + "commit) or a reference to a python dependency" ) - pull: bool = Field( - description="Whether to pull remote changes after checking out a branch", - default=False, + branch: str | SkipJsonSchema[None] = Field( + description="Branch name to create if checking out a reference not on a branch," + " to avoid leaving head detached", + default=None, )