From c5763f237ce49d0820b29fabb62e6d43acf2dff1 Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh <26975142+ZohebShaikh@users.noreply.github.com> Date: Wed, 22 Apr 2026 12:28:11 +0100 Subject: [PATCH 1/5] feat(scratch): Make scratch init faster --- src/blueapi/cli/scratch.py | 59 ++++++++++++++++++---------- tests/unit_tests/cli/test_scratch.py | 26 +++++++----- 2 files changed, 56 insertions(+), 29 deletions(-) diff --git a/src/blueapi/cli/scratch.py b/src/blueapi/cli/scratch.py index 85ad9310ce..311fd18037 100644 --- a/src/blueapi/cli/scratch.py +++ b/src/blueapi/cli/scratch.py @@ -3,6 +3,7 @@ import os import stat import textwrap +from concurrent.futures import ThreadPoolExecutor, as_completed from pathlib import Path from subprocess import Popen @@ -46,10 +47,23 @@ def setup_scratch( That is to prevent namespace clashing with the blueapi application. """) ) - for repo in config.repositories: - local_directory = config.root / repo.name - ensure_repo(repo.remote_url, local_directory, repo.branch) - scratch_install(local_directory, timeout=install_timeout) + with ThreadPoolExecutor() as executor: + futures = [ + executor.submit( + ensure_repo, repo.remote_url, config.root / repo.name, repo.branch + ) + for repo in config.repositories + ] + for future in as_completed(futures): + try: + future.result() + except Exception as e: + raise RuntimeError(f"Failed to clone repo \n{e}") + + scratch_install( + [config.root / repo.name for repo in config.repositories], + timeout=install_timeout, + ) def ensure_repo( @@ -69,7 +83,9 @@ def ensure_repo( if not local_directory.exists(): LOGGER.info(f"Cloning {remote_url}") - repo = Repo.clone_from(remote_url, local_directory) + repo = Repo.clone_from( + remote_url, local_directory, multi_options=["--filter=blob:none"] + ) LOGGER.info(f"Cloned {remote_url} -> {local_directory}") elif local_directory.is_dir(): repo = Repo(local_directory) @@ -93,34 +109,37 @@ def ensure_repo( local.checkout() -def scratch_install(path: Path, timeout: float = _DEFAULT_INSTALL_TIMEOUT) -> None: +def scratch_install( + paths: list[Path], timeout: float = _DEFAULT_INSTALL_TIMEOUT +) -> None: """ - Install a scratch package. Make blueapi aware of a repository checked out in + Install all scratch package. Make blueapi aware of a repository checked out in the scratch area. Make it automatically follow code changes to that repository (pending a restart). Do not install any of the package's dependencies as they may conflict with each other. Args: - path: Path to the checked out repository + paths: List of Path to the checked out repositories timeout: Time to wait for installation subprocess """ - - _validate_directory(path) - - LOGGER.info(f"Installing {path}") - process = Popen( - [ + if paths: + args = [ "uv", "pip", "install", "--no-deps", - "-e", - str(path), ] - ) - process.wait(timeout=timeout) - if process.returncode != 0: - raise RuntimeError(f"Failed to install {path}: Exit Code: {process.returncode}") + for path in paths: + _validate_directory(path) + args.extend(["-e", str(path)]) + + LOGGER.info("Installing packages") + process = Popen(args) + process.wait(timeout=timeout) + if process.returncode != 0: + raise RuntimeError( + f"Failed to install packages: Exit Code: {process.returncode}" + ) def _validate_root_directory(root_path: Path, required_gid: int | None) -> None: diff --git a/tests/unit_tests/cli/test_scratch.py b/tests/unit_tests/cli/test_scratch.py index 1c78401430..e1f7da94cf 100644 --- a/tests/unit_tests/cli/test_scratch.py +++ b/tests/unit_tests/cli/test_scratch.py @@ -63,7 +63,7 @@ def test_scratch_install_installs_path( mock_process.returncode = 0 mock_popen.return_value = mock_process - scratch_install(directory_path_with_sgid, timeout=1.0) + scratch_install([directory_path_with_sgid], timeout=1.0) mock_popen.assert_called_once_with( ["uv", "pip", "install", "--no-deps", "-e", str(directory_path_with_sgid)] @@ -72,12 +72,12 @@ def test_scratch_install_installs_path( def test_scratch_install_fails_on_file(file_path: Path): with pytest.raises(KeyError): - scratch_install(file_path, timeout=1.0) + scratch_install([file_path], timeout=1.0) def test_scratch_install_fails_on_nonexistant_path(nonexistant_path: Path): with pytest.raises(KeyError): - scratch_install(nonexistant_path, timeout=1.0) + scratch_install([nonexistant_path], timeout=1.0) @patch("blueapi.cli.scratch.Popen") @@ -92,7 +92,7 @@ def test_scratch_install_fails_on_non_zero_exit_code( mock_popen.return_value = mock_process with pytest.raises(RuntimeError): - scratch_install(directory_path_with_sgid, timeout=1.0) + scratch_install([directory_path_with_sgid], timeout=1.0) @patch("blueapi.cli.scratch.Repo") @@ -123,7 +123,9 @@ def test_repo_cloned_if_not_found_locally( ensure_repo("http://example.com/foo.git", nonexistant_path) mock_repo.assert_not_called() mock_repo.clone_from.assert_called_once_with( - "http://example.com/foo.git", nonexistant_path + "http://example.com/foo.git", + nonexistant_path, + multi_options=["--filter=blob:none"], ) @@ -140,7 +142,9 @@ def write_repo_files(): with file_path.open("w") as stream: stream.write("foo") - mock_repo.clone_from.side_effect = lambda url, path: write_repo_files() + mock_repo.clone_from.side_effect = lambda url, path, multi_options: ( + write_repo_files() + ) ensure_repo("http://example.com/foo.git", repo_root) assert file_path.exists() @@ -162,7 +166,9 @@ def test_cloned_repo_changes_to_new_branch(mock_repo, directory_path: Path): ensure_repo("http://example.com/foo.git", directory_path / "demo_branch", "demo") - mock_repo.clone_from.assert_called_once_with("http://example.com/foo.git", ANY) + mock_repo.clone_from.assert_called_once_with( + "http://example.com/foo.git", ANY, multi_options=["--filter=blob:none"] + ) repo.create_head.assert_called_once_with("demo", ANY) repo.create_head().checkout.assert_called_once() @@ -316,8 +322,10 @@ def test_setup_scratch_iterates_repos( mock_scratch_install.assert_has_calls( [ - call(directory_path_with_sgid / "foo", timeout=120.0), - call(directory_path_with_sgid / "bar", timeout=120.0), + call( + [directory_path_with_sgid / "foo", directory_path_with_sgid / "bar"], + timeout=120.0, + ), ] ) From f19f2255d3ed8988da2015fd1f3cd2d73b1f1356 Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh <26975142+ZohebShaikh@users.noreply.github.com> Date: Wed, 22 Apr 2026 13:02:06 +0100 Subject: [PATCH 2/5] fix tests and merge conflicts Co-authored-by: Copilot --- src/blueapi/cli/scratch.py | 15 ++++++++++----- tests/unit_tests/cli/test_scratch.py | 20 ++++++++++++++------ 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/src/blueapi/cli/scratch.py b/src/blueapi/cli/scratch.py index 0b102c584e..dc027af772 100644 --- a/src/blueapi/cli/scratch.py +++ b/src/blueapi/cli/scratch.py @@ -51,15 +51,18 @@ def setup_scratch( with ThreadPoolExecutor() as executor: futures = [ executor.submit( - ensure_repo, repo.remote_url, config.root / repo.name, repo.target_revision + ensure_repo, + repo.remote_url, + config.root / repo.name, + repo.target_revision, ) for repo in config.repositories ] for future in as_completed(futures): try: future.result() - except Exception as e: - raise RuntimeError(f"Failed to clone repo \n{e}") + except Exception as exc: + raise RuntimeError("Failed to clone repositories") from exc scratch_install( [config.root / repo.name for repo in config.repositories], @@ -67,7 +70,6 @@ def setup_scratch( ) - def ensure_repo( remote_url: str, local_directory: Path, target_revision: str | None = None ) -> None: @@ -86,7 +88,10 @@ def ensure_repo( if not local_directory.exists(): LOGGER.info(f"Cloning {remote_url}") repo = Repo.clone_from( - remote_url, local_directory, branch=target_revision ,multi_options=["--filter=blob:none"] + remote_url, + local_directory, + branch=target_revision, + multi_options=["--filter=blob:none"], ) LOGGER.info(f"Cloned {remote_url} -> {local_directory}") elif local_directory.is_dir(): diff --git a/tests/unit_tests/cli/test_scratch.py b/tests/unit_tests/cli/test_scratch.py index 0655900b10..e0e62382c8 100644 --- a/tests/unit_tests/cli/test_scratch.py +++ b/tests/unit_tests/cli/test_scratch.py @@ -123,7 +123,10 @@ def test_repo_cloned_if_not_found_locally( ensure_repo("http://example.com/foo.git", nonexistant_path) mock_repo.assert_not_called() mock_repo.clone_from.assert_called_once_with( - "http://example.com/foo.git", nonexistant_path, branch=None,multi_options=["--filter=blob:none"] + "http://example.com/foo.git", + nonexistant_path, + branch=None, + multi_options=["--filter=blob:none"], ) @@ -140,7 +143,9 @@ def write_repo_files(): with file_path.open("w") as stream: stream.write("foo") - mock_repo.clone_from.side_effect = lambda url, path, branch=None, multi_options: write_repo_files() + mock_repo.clone_from.side_effect = lambda url, path, branch, multi_options: ( + write_repo_files() + ) ensure_repo("http://example.com/foo.git", repo_root) assert file_path.exists() @@ -163,10 +168,11 @@ def test_cloned_repo_changes_to_new_branch(mock_repo, directory_path: Path): ensure_repo("http://example.com/foo.git", directory_path / "demo_branch", "demo") mock_repo.clone_from.assert_called_once_with( - "http://example.com/foo.git", ANY,branch="demo", multi_options=["--filter=blob:none"] + "http://example.com/foo.git", + ANY, + branch="demo", + multi_options=["--filter=blob:none"], ) - repo.create_head.assert_called_once_with("demo", ANY) - repo.create_head().checkout.assert_called_once() @patch("blueapi.cli.scratch.Repo") @@ -380,7 +386,9 @@ def test_setup_scratch_continues_after_failure( ], ) mock_ensure_repo.side_effect = [None, RuntimeError("bar"), None] - with pytest.raises(RuntimeError, match="bar"): + with pytest.raises( + RuntimeError, match="Failed to clone", check=lambda e: str(e.__cause__) == "bar" + ): setup_scratch(config) From 6539d6029e89374bca5c218392b31b22d0470131 Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh <26975142+ZohebShaikh@users.noreply.github.com> Date: Thu, 23 Apr 2026 09:42:22 +0100 Subject: [PATCH 3/5] Add code review changes --- src/blueapi/cli/scratch.py | 51 ++++++++++++++-------------- tests/unit_tests/cli/test_scratch.py | 17 +++++----- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/blueapi/cli/scratch.py b/src/blueapi/cli/scratch.py index dc027af772..1c6aef5fe7 100644 --- a/src/blueapi/cli/scratch.py +++ b/src/blueapi/cli/scratch.py @@ -65,7 +65,7 @@ def setup_scratch( raise RuntimeError("Failed to clone repositories") from exc scratch_install( - [config.root / repo.name for repo in config.repositories], + *(config.root / repo.name for repo in config.repositories), timeout=install_timeout, ) @@ -91,7 +91,7 @@ def ensure_repo( remote_url, local_directory, branch=target_revision, - multi_options=["--filter=blob:none"], + filter="blob:none", ) LOGGER.info(f"Cloned {remote_url} -> {local_directory}") elif local_directory.is_dir(): @@ -116,37 +116,36 @@ def ensure_repo( ) -def scratch_install( - paths: list[Path], timeout: float = _DEFAULT_INSTALL_TIMEOUT -) -> None: +def scratch_install(*paths: Path, timeout: float = _DEFAULT_INSTALL_TIMEOUT) -> None: """ - Install all scratch package. Make blueapi aware of a repository checked out in - the scratch area. Make it automatically follow code changes to that repository + Install all scratch packages. Make blueapi aware of repositories checked out in + the scratch area. Make it automatically follow code changes to that repositories (pending a restart). Do not install any of the package's dependencies as they may conflict with each other. Args: - paths: List of Path to the checked out repositories + paths: List of Paths to the checked out repositories timeout: Time to wait for installation subprocess """ - if paths: - args = [ - "uv", - "pip", - "install", - "--no-deps", - ] - for path in paths: - _validate_directory(path) - args.extend(["-e", str(path)]) - - LOGGER.info("Installing packages") - process = Popen(args) - process.wait(timeout=timeout) - if process.returncode != 0: - raise RuntimeError( - f"Failed to install packages: Exit Code: {process.returncode}" - ) + if not paths: + return + args = [ + "uv", + "pip", + "install", + "--no-deps", + ] + for path in paths: + _validate_directory(path) + args.extend(["-e", str(path)]) + + LOGGER.info("Installing packages") + process = Popen(args) + process.wait(timeout=timeout) + if process.returncode != 0: + raise RuntimeError( + f"Failed to install packages: Exit Code: {process.returncode}" + ) def _validate_root_directory(root_path: Path, required_gid: int | None) -> None: diff --git a/tests/unit_tests/cli/test_scratch.py b/tests/unit_tests/cli/test_scratch.py index e0e62382c8..7252f78267 100644 --- a/tests/unit_tests/cli/test_scratch.py +++ b/tests/unit_tests/cli/test_scratch.py @@ -63,7 +63,7 @@ def test_scratch_install_installs_path( mock_process.returncode = 0 mock_popen.return_value = mock_process - scratch_install([directory_path_with_sgid], timeout=1.0) + scratch_install(directory_path_with_sgid, timeout=1.0) mock_popen.assert_called_once_with( ["uv", "pip", "install", "--no-deps", "-e", str(directory_path_with_sgid)] @@ -72,12 +72,12 @@ def test_scratch_install_installs_path( def test_scratch_install_fails_on_file(file_path: Path): with pytest.raises(KeyError): - scratch_install([file_path], timeout=1.0) + scratch_install(file_path, timeout=1.0) def test_scratch_install_fails_on_nonexistant_path(nonexistant_path: Path): with pytest.raises(KeyError): - scratch_install([nonexistant_path], timeout=1.0) + scratch_install(nonexistant_path, timeout=1.0) @patch("blueapi.cli.scratch.Popen") @@ -92,7 +92,7 @@ def test_scratch_install_fails_on_non_zero_exit_code( mock_popen.return_value = mock_process with pytest.raises(RuntimeError): - scratch_install([directory_path_with_sgid], timeout=1.0) + scratch_install(directory_path_with_sgid, timeout=1.0) @patch("blueapi.cli.scratch.Repo") @@ -126,7 +126,7 @@ def test_repo_cloned_if_not_found_locally( "http://example.com/foo.git", nonexistant_path, branch=None, - multi_options=["--filter=blob:none"], + filter="blob:none", ) @@ -143,7 +143,7 @@ def write_repo_files(): with file_path.open("w") as stream: stream.write("foo") - mock_repo.clone_from.side_effect = lambda url, path, branch, multi_options: ( + mock_repo.clone_from.side_effect = lambda url, path, branch, filter: ( write_repo_files() ) @@ -171,7 +171,7 @@ def test_cloned_repo_changes_to_new_branch(mock_repo, directory_path: Path): "http://example.com/foo.git", ANY, branch="demo", - multi_options=["--filter=blob:none"], + filter="blob:none", ) @@ -354,7 +354,8 @@ def test_setup_scratch_iterates_repos( mock_scratch_install.assert_has_calls( [ call( - [directory_path_with_sgid / "foo", directory_path_with_sgid / "bar"], + directory_path_with_sgid / "foo", + directory_path_with_sgid / "bar", timeout=120.0, ), ] From 0a0e6e954feb75ac6612a9d934deaeb7f53570f3 Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh <26975142+ZohebShaikh@users.noreply.github.com> Date: Thu, 23 Apr 2026 11:14:58 +0100 Subject: [PATCH 4/5] remove all --- src/blueapi/cli/scratch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/blueapi/cli/scratch.py b/src/blueapi/cli/scratch.py index 1c6aef5fe7..a6dd194376 100644 --- a/src/blueapi/cli/scratch.py +++ b/src/blueapi/cli/scratch.py @@ -118,7 +118,7 @@ def ensure_repo( def scratch_install(*paths: Path, timeout: float = _DEFAULT_INSTALL_TIMEOUT) -> None: """ - Install all scratch packages. Make blueapi aware of repositories checked out in + Install scratch packages. Make blueapi aware of repositories checked out in the scratch area. Make it automatically follow code changes to that repositories (pending a restart). Do not install any of the package's dependencies as they may conflict with each other. From c1f26a23cbf5dc2abd572cb3150ae06e86cac979 Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh <26975142+ZohebShaikh@users.noreply.github.com> Date: Thu, 23 Apr 2026 11:32:31 +0100 Subject: [PATCH 5/5] fix typos --- src/blueapi/cli/scratch.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/blueapi/cli/scratch.py b/src/blueapi/cli/scratch.py index a6dd194376..c2fecd1e18 100644 --- a/src/blueapi/cli/scratch.py +++ b/src/blueapi/cli/scratch.py @@ -87,7 +87,7 @@ def ensure_repo( if not local_directory.exists(): LOGGER.info(f"Cloning {remote_url}") - repo = Repo.clone_from( + Repo.clone_from( remote_url, local_directory, branch=target_revision, @@ -119,8 +119,8 @@ def ensure_repo( def scratch_install(*paths: Path, timeout: float = _DEFAULT_INSTALL_TIMEOUT) -> None: """ Install scratch packages. Make blueapi aware of repositories checked out in - the scratch area. Make it automatically follow code changes to that repositories - (pending a restart). Do not install any of the package's dependencies as they + the scratch area. Make it automatically follow code changes to those repositories + (pending a restart). Do not install any of the packages' dependencies as they may conflict with each other. Args: