Merge branch 'main' into Improve-BlueapiClient-plan-repr-to-add-parameter-types

Author: oliwenmandiamond

.github/workflows/_system_test.yml

@@ -22,7 +22,7 @@ jobs:
         uses: astral-sh/setup-uv@v7
 
       - name: Compose services
-        uses: hoverkraft-tech/compose-action@4894d2492015c1774ee5a13a95b1072093087ec3 # v2.5.0
+        uses: hoverkraft-tech/compose-action@d2bee4f07e8ca410d6b196d00f90c12e7d48c33a # v2.6.0
         with:
           compose-file: "tests/system_tests/compose.yaml"
 

.github/workflows/codeql.yml

@@ -62,7 +62,7 @@ jobs:
 
       # Initializes the CodeQL tools for scanning.
       - name: Initialize CodeQL
-        uses: github/codeql-action/init@c10b8064de6f491fea524254123dbe5e09572f13 # v4
+        uses: github/codeql-action/init@95e58e9a2cdfd71adc6e0353d5c52f41a045d225 # v4
         with:
           languages: ${{ matrix.language }}
           build-mode: ${{ matrix.build-mode }}
@@ -89,6 +89,6 @@ jobs:
           echo '  make release'
           exit 1
       - name: Perform CodeQL Analysis
-        uses: github/codeql-action/analyze@c10b8064de6f491fea524254123dbe5e09572f13 # v4
+        uses: github/codeql-action/analyze@95e58e9a2cdfd71adc6e0353d5c52f41a045d225 # v4
         with:
           category: "/language:${{matrix.language}}"

Dockerfile

@@ -47,7 +47,7 @@ RUN DEBIAN_FRONTEND=noninteractive apt-get update && apt-get install -y --no-ins
     && apt-get dist-clean
 
 # Install uv to allow setup-scratch to run
-COPY --from=ghcr.io/astral-sh/uv:0.10 /uv /uvx /bin/
+COPY --from=ghcr.io/astral-sh/uv:0.11 /uv /uvx /bin/
 
 # For this pod to understand finding user information from LDAP
 RUN sed -i 's/files/ldap files/g' /etc/nsswitch.conf
@@ -64,7 +64,7 @@ COPY --from=build /python /python
 
 # Copy the environment, but not the source code
 COPY --chown=1000:1000 --from=build /app/.venv /app/.venv
-RUN chmod o+wrX /app/.venv
+RUN chmod -R 777 /app
 ENV PATH=/app/.venv/bin:$PATH
 
 # Add copy of blueapi source to container for debugging

codecov.yml

@@ -4,5 +4,3 @@ coverage:
       default:
         target: 85% # the required coverage value
         threshold: 1% # the leniency in hitting the target
-ignore:
-  - "src/blueapi/startup"

helm/blueapi/config_schema.json

@@ -430,9 +430,9 @@
                     "title": "Remote Url",
                     "type": "string"
                 },
-                "branch": {
-                    "description": "Branch of repo to check out - defaults to remote's default when cloning and the existing branch when the repo already exists",
-                    "title": "Branch",
+                "target_revision": {
+                    "description": "Revision (branch or tag) to check out when cloning - defaults to remote's HEAD. If a tag is used, the repo will be left in a 'detached head' state.",
+                    "title": "Target Revision",
                     "type": "string"
                 }
             },

helm/blueapi/values.schema.json

@@ -852,11 +852,6 @@
             "title": "ScratchRepository",
             "type": "object",
             "properties": {
-                "branch": {
-                    "title": "Branch",
-                    "description": "Branch of repo to check out - defaults to remote's default when cloning and the existing branch when the repo already exists",
-                    "type": "string"
-                },
                 "name": {
                     "title": "Name",
                     "description": "Unique name for this repository in the scratch directory",
@@ -868,6 +863,11 @@
                     "description": "URL to clone from",
                     "default": "https://github.com/example/example.git",
                     "type": "string"
+                },
+                "target_revision": {
+                    "title": "Target Revision",
+                    "description": "Revision (branch or tag) to check out when cloning - defaults to remote's HEAD. If a tag is used, the repo will be left in a 'detached head' state.",
+                    "type": "string"
                 }
             },
             "additionalProperties": false

pyproject.toml

@@ -55,6 +55,7 @@ dev = [
     "pytest-cov",
     "pytest-asyncio",
     "pytest-httpx>=0.35.0",
+    "pytest-timeout",
     "responses",
     "ruff",
     "semver",
@@ -72,7 +73,7 @@ dev = [
     "jwcrypto",
     "deepdiff",
     "tiled[minimal-server]>=0.2.4", # For system-test of dls.py
-    "respx"
+    "respx",
 ]
 
 [project.scripts]
@@ -94,20 +95,17 @@ reportMissingImports = false  # Ignore missing stubs in imported modules
 
 [tool.pytest.ini_options]
 # Run pytest with all our checkers, and don't spam us with massive tracebacks on error
-addopts = """
-    --tb=native -vv --doctest-modules --doctest-glob="*.rst"
-    --ignore=src/blueapi/startup
-    """
+addopts = '--tb=native -vv --doctest-modules --doctest-glob="*.rst"'
 # https://iscinumpy.gitlab.io/post/bound-version-constraints/#watch-for-warnings
 filterwarnings = ["error", "ignore::DeprecationWarning"]
 # Doctest python code in docs, python code in src docstrings, test functions in tests
 testpaths = "docs src tests"
 asyncio_mode = "auto"
+timeout = 3
 
 [tool.coverage.run]
 patch = ["subprocess"]
 data_file = "/tmp/blueapi.coverage"
-omit = ["src/blueapi/startup/**/*"]
 
 [tool.coverage.paths]
 # Tests are run from installed location, map back to the src directory
@@ -178,6 +176,7 @@ commands = [
         "term",
         "--cov-report",
         "xml:cov.xml",
+        "--timeout=60",
         { replace = "posargs", default = [
         ], extend = true },
     ],

src/blueapi/cli/cli.py

@@ -26,6 +26,7 @@
 from blueapi.client.rest import (
     BlueskyRemoteControlError,
     InvalidParametersError,
+    NonJsonResponseError,
     ServiceUnavailableError,
     UnauthorisedAccessError,
     UnknownPlanError,
@@ -83,8 +84,24 @@ def is_str_dict(val: Any) -> TypeGuard[TaskParameters]:
 @click.option(
     "-c", "--config", type=Path, help="Path to configuration YAML file", multiple=True
 )
+@click.option(
+    "-v",
+    "--verbose",
+    "log_level",
+    flag_value="DEBUG",
+    help="Include DEBUG level logging output",
+)
+@click.option(
+    "-q",
+    "--quiet",
+    "log_level",
+    flag_value="ERROR",
+    help="Reduce logging noise to only show errors",
+)
 @click.pass_context
-def main(ctx: click.Context, config: tuple[Path, ...]) -> None:
+def main(
+    ctx: click.Context, config: tuple[Path, ...], log_level: str | None = None
+) -> None:
     # if no command is supplied, run with the options passed
 
     # Set umask to DLS standard
@@ -96,6 +113,9 @@ def main(ctx: click.Context, config: tuple[Path, ...]) -> None:
     except FileNotFoundError as fnfe:
         raise ClickException(f"Config file not found: {fnfe.filename}") from fnfe
 
+    if log_level:
+        config_loader.use_values({"logging": {"level": log_level}})
+
     loaded_config: ApplicationConfig = config_loader.load()
 
     set_up_logging(loaded_config.logging)
@@ -494,13 +514,16 @@ def login(obj: dict) -> None:
         print("Logged in")
     except Exception:
         client = BlueapiClient.from_config(config)
-        if oidc := client.oidc_config:
-            auth = SessionManager(
-                oidc, cache_manager=SessionCacheManager(config.auth_token_path)
-            )
-            auth.start_device_flow()
-        else:
-            print("Server is not configured to use authentication!")
+        try:
+            if oidc := client.oidc_config:
+                auth = SessionManager(
+                    oidc, cache_manager=SessionCacheManager(config.auth_token_path)
+                )
+                auth.start_device_flow()
+            else:
+                print("Server is not configured to use authentication!")
+        except NonJsonResponseError as e:
+            print(str(e))
 
 
 @main.command(name="logout")

src/blueapi/cli/scratch.py

@@ -48,12 +48,12 @@ def setup_scratch(
             )
     for repo in config.repositories:
         local_directory = config.root / repo.name
-        ensure_repo(repo.remote_url, local_directory, repo.branch)
+        ensure_repo(repo.remote_url, local_directory, repo.target_revision)
         scratch_install(local_directory, timeout=install_timeout)
 
 
 def ensure_repo(
-    remote_url: str, local_directory: Path, branch: str | None = None
+    remote_url: str, local_directory: Path, target_revision: str | None = None
 ) -> None:
     """
     Ensure that a repository is checked out for use in the scratch area.
@@ -69,29 +69,29 @@ def ensure_repo(
 
     if not local_directory.exists():
         LOGGER.info(f"Cloning {remote_url}")
-        repo = Repo.clone_from(remote_url, local_directory)
+        Repo.clone_from(remote_url, local_directory, branch=target_revision)
         LOGGER.info(f"Cloned {remote_url} -> {local_directory}")
     elif local_directory.is_dir():
         repo = Repo(local_directory)
-        LOGGER.info(f"Found {local_directory}")
+        head = repo.head.commit
+        LOGGER.info("Found %s @ %s", local_directory, head.name_rev)
+        if target_revision:
+            if target_revision in repo.refs:
+                if repo.refs[target_revision].commit != head:
+                    LOGGER.warning(
+                        "Repository %s not at target revision: %r instead of %r",
+                        local_directory.name,
+                        head.name_rev,
+                        target_revision,
+                    )
+            else:
+                LOGGER.warning("Target revision %r not found", target_revision)
     else:
         raise KeyError(
-            f"Unable to open {local_directory} as a git repository because it is a file"
+            f"Unable to open {local_directory} as a git repository because it is not a "
+            "directory"
         )
 
-    if branch:
-        if not (local := getattr(repo.heads, branch, None)):
-            origin = repo.remotes[0]
-            origin.fetch()
-            LOGGER.info(
-                "Creating branch '%s' to track remote '%s'", branch, origin.refs[branch]
-            )
-            local = repo.create_head(branch, origin.refs[branch])
-            local.set_tracking_branch(origin.refs[branch])
-
-        LOGGER.info("Checking out branch '%s'", branch)
-        local.checkout()
-
 
 def scratch_install(path: Path, timeout: float = _DEFAULT_INSTALL_TIMEOUT) -> None:
     """

src/blueapi/client/client.py

@@ -37,7 +37,7 @@
 )
 from blueapi.utils import deprecated
 from blueapi.worker import WorkerEvent, WorkerState
-from blueapi.worker.event import ProgressEvent, TaskStatus
+from blueapi.worker.event import ProgressEvent, TaskError, TaskResult, TaskStatus
 from blueapi.worker.task_worker import TrackableTask
 
 from .event_bus import AnyEvent, EventBusClient, OnAnyEvent
@@ -166,13 +166,17 @@ def __init__(self, name, model: PlanModel, client: "BlueapiClient"):
         self._client = client
         self.__doc__ = model.description
 
-    def __call__(self, *args, **kwargs):
+    def __call__(self, *args, **kwargs) -> Any:
         req = TaskRequest(
             name=self.name,
             params=self._build_args(*args, **kwargs),
             instrument_session=self._client.instrument_session,
         )
-        self._client.run_task(req)
+        match self._client.run_task(req):
+            case TaskStatus(result=TaskResult(result=res)):
+                return res
+            case TaskStatus(result=TaskError(type=typ, message=msg)):
+                raise PlanFailedError(typ, msg)
 
     @property
     def help_text(self) -> str:
@@ -795,3 +799,9 @@ def login(self, token_path: Path | None = None):
                 auth.start_device_flow()
             else:
                 print("Server is not configured to use authentication!")
+
+
+class PlanFailedError(Exception):
+    def __init__(self, typ: str, message: str):
+        super().__init__(message)
+        self._type = typ