From 0b40ea3fb9275d9d9d64867024452b711f6c4213 Mon Sep 17 00:00:00 2001 From: Ambient Code Bot Date: Thu, 9 Apr 2026 15:56:42 +0000 Subject: [PATCH 1/6] Remove opendal dependency from QEMU driver Move FlasherInterface to core jumpstarter package (jumpstarter.driver.flasher) and create a simplified FlasherClient (jumpstarter.client.flasher) that streams local files via resource_async and passes HTTP URLs as PresignedRequestResource for exporter-side download, eliminating the need for the opendal library. Update QEMU driver to import from the new core location and remove jumpstarter-driver-opendal from its dependencies. Update tests to use direct HTTP URLs instead of opendal Operator. Closes #441 Co-Authored-By: Claude Opus 4.6 --- .../jumpstarter_driver_qemu/driver.py | 3 +- .../jumpstarter_driver_qemu/driver_test.py | 4 +- .../jumpstarter-driver-qemu/pyproject.toml | 2 - .../jumpstarter/client/__init__.py | 3 +- .../jumpstarter/jumpstarter/client/flasher.py | 286 ++++++++++++++++++ .../jumpstarter/driver/__init__.py | 3 +- .../jumpstarter/jumpstarter/driver/flasher.py | 22 ++ 7 files changed, 314 insertions(+), 9 deletions(-) create mode 100644 python/packages/jumpstarter/jumpstarter/client/flasher.py create mode 100644 python/packages/jumpstarter/jumpstarter/driver/flasher.py diff --git a/python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.py b/python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.py index 833f7209c..9bd760d50 100644 --- a/python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.py +++ b/python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.py @@ -18,14 +18,13 @@ from anyio import fail_after, run_process, sleep from anyio.streams.file import FileReadStream, FileWriteStream from jumpstarter_driver_network.driver import TcpNetwork, UnixNetwork, VsockNetwork -from jumpstarter_driver_opendal.driver import FlasherInterface from jumpstarter_driver_power.driver import PowerInterface, PowerReading from jumpstarter_driver_pyserial.driver import PySerial from pydantic import BaseModel, ByteSize, Field, TypeAdapter, ValidationError, validate_call from qemu.qmp import QMPClient from qemu.qmp.protocol import ConnectError, Runstate -from jumpstarter.driver import Driver, export +from jumpstarter.driver import Driver, FlasherInterface, export from jumpstarter.streams.encoding import AutoDecompressIterator diff --git a/python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver_test.py b/python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver_test.py index c9246e504..1055a8d2c 100644 --- a/python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver_test.py +++ b/python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver_test.py @@ -9,7 +9,6 @@ import pytest import requests -from opendal import Operator from jumpstarter_driver_qemu.driver import Qemu @@ -77,8 +76,7 @@ def test_driver_qemu(tmp_path, ovmf): qemu.flasher.flash(cached_image.resolve()) else: qemu.flasher.flash( - f"pub/fedora/linux/releases/43/Cloud/{arch}/images/Fedora-Cloud-Base-Generic-43-1.6.{arch}.qcow2", - operator=Operator("http", endpoint="https://download.fedoraproject.org"), + f"https://download.fedoraproject.org/pub/fedora/linux/releases/43/Cloud/{arch}/images/Fedora-Cloud-Base-Generic-43-1.6.{arch}.qcow2", ) qemu.power.on() diff --git a/python/packages/jumpstarter-driver-qemu/pyproject.toml b/python/packages/jumpstarter-driver-qemu/pyproject.toml index d62d18c9d..e3c413876 100644 --- a/python/packages/jumpstarter-driver-qemu/pyproject.toml +++ b/python/packages/jumpstarter-driver-qemu/pyproject.toml @@ -14,7 +14,6 @@ dependencies = [ "jumpstarter", "jumpstarter-driver-composite", "jumpstarter-driver-network", - "jumpstarter-driver-opendal", "jumpstarter-driver-power", "jumpstarter-driver-pyserial", "pyyaml>=6.0.2", @@ -38,7 +37,6 @@ source = "vcs" raw-options = { 'root' = '../../../' } [tool.uv.sources] -jumpstarter-driver-opendal = { workspace = true } jumpstarter-driver-composite = { workspace = true } jumpstarter-driver-network = { workspace = true } jumpstarter-driver-pyserial = { workspace = true } diff --git a/python/packages/jumpstarter/jumpstarter/client/__init__.py b/python/packages/jumpstarter/jumpstarter/client/__init__.py index ae9ac7912..62dbf689c 100644 --- a/python/packages/jumpstarter/jumpstarter/client/__init__.py +++ b/python/packages/jumpstarter/jumpstarter/client/__init__.py @@ -1,5 +1,6 @@ from .base import DriverClient from .client import client_from_path +from .flasher import FlasherClient, FlasherClientInterface from .lease import DirectLease, Lease -__all__ = ["DriverClient", "DirectLease", "client_from_path", "Lease"] +__all__ = ["DriverClient", "DirectLease", "FlasherClient", "FlasherClientInterface", "client_from_path", "Lease"] diff --git a/python/packages/jumpstarter/jumpstarter/client/flasher.py b/python/packages/jumpstarter/jumpstarter/client/flasher.py new file mode 100644 index 000000000..7c101b7e6 --- /dev/null +++ b/python/packages/jumpstarter/jumpstarter/client/flasher.py @@ -0,0 +1,286 @@ +""" +Simplified FlasherClient that does not depend on opendal. + +For local files: streams via the existing resource_async mechanism using anyio. +For HTTP URLs: passes a PresignedRequestResource directly to the exporter, +which already handles presigned downloads via aiohttp. +""" + +from __future__ import annotations + +from abc import ABCMeta, abstractmethod +from contextlib import asynccontextmanager +from dataclasses import dataclass, field +from os import PathLike +from pathlib import Path +from typing import Any, Callable, Mapping, cast + +import click +from anyio import BrokenResourceError, EndOfStream +from anyio.abc import ObjectStream + +from jumpstarter.client import DriverClient +from jumpstarter.client.adapters import blocking +from jumpstarter.client.decorators import driver_click_group +from jumpstarter.common.resources import PresignedRequestResource +from jumpstarter.streams.encoding import Compression +from jumpstarter.streams.progress import ProgressAttribute + +PathBuf = str | PathLike + + +@dataclass(kw_only=True) +class _AsyncIteratorStream(ObjectStream[bytes]): + """Wraps an async iterator as an ObjectStream for resource_async.""" + + iterator: Any + total: int | None = None + + async def receive(self) -> bytes: + try: + return await self.iterator.__anext__() + except StopAsyncIteration: + raise EndOfStream from None + + async def send(self, item: bytes): + raise BrokenResourceError("read-only stream") + + async def send_eof(self): + pass + + async def aclose(self): + pass + + @property + def extra_attributes(self) -> Mapping[Any, Callable[[], Any]]: + if self.total is not None and self.total > 0: + return {ProgressAttribute.total: lambda: float(self.total)} + return {} + + +@dataclass(kw_only=True) +class _FileWriteObjectStream(ObjectStream[bytes]): + """Wraps a file path as a writable ObjectStream for resource_async.""" + + path: Path + _file: Any = field(default=None, init=False) + + async def receive(self) -> bytes: + raise EndOfStream + + async def send(self, item: bytes): + if self._file is None: + import anyio + + self._file = await anyio.open_file(self.path, "wb") + await self._file.write(item) + + async def send_eof(self): + if self._file is not None: + await self._file.aclose() + self._file = None + + async def aclose(self): + if self._file is not None: + await self._file.aclose() + self._file = None + + +def _parse_path(path: PathBuf) -> tuple[Path | None, str | None]: + """Parse a path into either a local Path or an HTTP URL. + + Returns (local_path, None) for local files, or (None, url) for HTTP URLs. + """ + path_str = str(path) + if path_str.startswith(("http://", "https://")): + return None, path_str + return Path(path).resolve(), None + + +@blocking +@asynccontextmanager +async def _local_file_adapter( + *, + client: DriverClient, + path: Path, + mode: str = "rb", + compression: Compression | None = None, +): + """Stream a local file via resource_async, without opendal.""" + import anyio + + if mode == "rb": + # Read mode: stream file content to exporter + file_size = path.stat().st_size + + async def file_reader(): + async with await anyio.open_file(path, "rb") as f: + while True: + chunk = await f.read(65536) + if not chunk: + break + yield chunk + + stream = _AsyncIteratorStream( + iterator=file_reader(), + total=file_size, + ) + + async with client.resource_async(stream, content_encoding=compression) as res: + yield res + else: + # Write mode: receive content from exporter into file + stream = _FileWriteObjectStream(path=path) + async with client.resource_async(stream, content_encoding=compression) as res: + yield res + + +@blocking +@asynccontextmanager +async def _http_url_adapter( + *, + client: DriverClient, + url: str, + mode: str = "rb", +): + """Create a PresignedRequestResource for an HTTP URL. + + The exporter already handles HTTP downloads via aiohttp, + so we just pass the URL as a presigned GET request. + """ + if mode == "rb": + yield PresignedRequestResource( + headers={}, + url=url, + method="GET", + ).model_dump(mode="json") + else: + yield PresignedRequestResource( + headers={}, + url=url, + method="PUT", + ).model_dump(mode="json") + + +class FlasherClientInterface(metaclass=ABCMeta): + @abstractmethod + def flash( + self, + path: PathBuf | dict[str, PathBuf], + *, + target: str | None = None, + compression: Compression | None = None, + ): + """Flash image to DUT""" + ... + + @abstractmethod + def dump( + self, + path: PathBuf, + *, + target: str | None = None, + compression: Compression | None = None, + ): + """Dump image from DUT""" + ... + + def cli(self): + @driver_click_group(self) + def base(): + """Generic flasher interface""" + pass + + @base.command() + @click.argument("file", nargs=-1, required=False) + @click.option( + "--target", + "-t", + "target_specs", + multiple=True, + help="name:file", + ) + @click.option("--compression", type=click.Choice(Compression, case_sensitive=False)) + def flash(file, target_specs, compression): + if target_specs: + mapping: dict[str, str] = {} + for spec in target_specs: + if ":" not in spec: + raise click.ClickException(f"Invalid target spec '{spec}', expected name:file") + name, img = spec.split(":", 1) + mapping[name] = img + self.flash(cast(dict[str, PathBuf], mapping), compression=compression) + return + + if not file: + raise click.ClickException("FILE argument is required unless --target/-t is used") + + self.flash(file[0], target=None, compression=compression) + + @base.command() + @click.argument("file") + @click.option("--target", type=str) + @click.option("--compression", type=click.Choice(Compression, case_sensitive=False)) + def dump(file, target, compression): + """Dump image from DUT to file""" + self.dump(file, target=target, compression=compression) + + return base + + +class FlasherClient(FlasherClientInterface, DriverClient): + def _flash_single( + self, + image: PathBuf, + *, + target: str | None, + compression: Compression | None, + ): + """Flash image to DUT""" + local_path, url = _parse_path(image) + + if url is not None: + # HTTP URL: pass as presigned request for exporter-side download + with _http_url_adapter(client=self, url=url, mode="rb") as handle: + return self.call("flash", handle, target) + else: + # Local file: stream via resource_async + with _local_file_adapter(client=self, path=local_path, mode="rb", compression=compression) as handle: + return self.call("flash", handle, target) + + def flash( + self, + path: PathBuf | dict[str, PathBuf], + *, + target: str | None = None, + compression: Compression | None = None, + ): + if isinstance(path, dict): + if target is not None: + from jumpstarter.common.exceptions import ArgumentError + + raise ArgumentError("'target' parameter is not valid when flashing multiple images") + + results: dict[str, object] = {} + for part, img in path.items(): + results[part] = self._flash_single(img, target=part, compression=compression) + return results + + return self._flash_single(path, target=target, compression=compression) + + def dump( + self, + path: PathBuf, + *, + target: str | None = None, + compression: Compression | None = None, + ): + """Dump image from DUT""" + local_path, url = _parse_path(path) + + if url is not None: + with _http_url_adapter(client=self, url=url, mode="wb") as handle: + return self.call("dump", handle, target) + else: + with _local_file_adapter(client=self, path=local_path, mode="wb", compression=compression) as handle: + return self.call("dump", handle, target) diff --git a/python/packages/jumpstarter/jumpstarter/driver/__init__.py b/python/packages/jumpstarter/jumpstarter/driver/__init__.py index 01c8e388e..ac7400106 100644 --- a/python/packages/jumpstarter/jumpstarter/driver/__init__.py +++ b/python/packages/jumpstarter/jumpstarter/driver/__init__.py @@ -1,4 +1,5 @@ from .base import Driver from .decorators import export, exportstream +from .flasher import FlasherInterface -__all__ = ["Driver", "export", "exportstream"] +__all__ = ["Driver", "FlasherInterface", "export", "exportstream"] diff --git a/python/packages/jumpstarter/jumpstarter/driver/flasher.py b/python/packages/jumpstarter/jumpstarter/driver/flasher.py new file mode 100644 index 000000000..47d655875 --- /dev/null +++ b/python/packages/jumpstarter/jumpstarter/driver/flasher.py @@ -0,0 +1,22 @@ +""" +Common flasher interface for drivers that flash images to devices. + +This is a pure ABC with no external dependencies, providing a common interface +for flasher drivers across the jumpstarter ecosystem. +""" + +from __future__ import annotations + +from abc import ABCMeta, abstractmethod + + +class FlasherInterface(metaclass=ABCMeta): + @classmethod + def client(cls) -> str: + return "jumpstarter.client.flasher.FlasherClient" + + @abstractmethod + def flash(self, source, target: str | None = None): ... + + @abstractmethod + def dump(self, target, partition: str | None = None): ... From 0c8625e3db4bb9ac330f71308dae09a9603f96ab Mon Sep 17 00:00:00 2001 From: "ambient-code[bot]" <178348098+ambient-code[bot]@users.noreply.github.com> Date: Thu, 9 Apr 2026 16:32:15 +0000 Subject: [PATCH 2/6] Add click dependency to jumpstarter package The new FlasherClient in jumpstarter/client/flasher.py imports click but it was not listed in the dependencies, causing import errors in tests. Co-Authored-By: Claude Opus 4.6 --- python/packages/jumpstarter/pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/python/packages/jumpstarter/pyproject.toml b/python/packages/jumpstarter/pyproject.toml index a09ce03f2..9a9275076 100644 --- a/python/packages/jumpstarter/pyproject.toml +++ b/python/packages/jumpstarter/pyproject.toml @@ -21,6 +21,7 @@ dependencies = [ "rich>=14.0.0", "tenacity>=8.2.0", "backports-zstd>=1.1.0 ; python_full_version < '3.14'", + "click>=8.1.7.2", ] [dependency-groups] From 25c6f5cb46c6dd95ca7f285b0c24139deeba2c22 Mon Sep 17 00:00:00 2001 From: Ambient Code Bot Date: Fri, 10 Apr 2026 12:33:08 +0000 Subject: [PATCH 3/6] Add unit tests for FlasherClient URL routing and path parsing Ensures the HTTP URL code path in FlasherClient is covered by tests, since the integration test only exercises this path when no cached image is available. Co-Authored-By: Claude Opus 4.6 --- .../jumpstarter/client/flasher_test.py | 168 ++++++++++++++++++ 1 file changed, 168 insertions(+) create mode 100644 python/packages/jumpstarter/jumpstarter/client/flasher_test.py diff --git a/python/packages/jumpstarter/jumpstarter/client/flasher_test.py b/python/packages/jumpstarter/jumpstarter/client/flasher_test.py new file mode 100644 index 000000000..d2b62cb7f --- /dev/null +++ b/python/packages/jumpstarter/jumpstarter/client/flasher_test.py @@ -0,0 +1,168 @@ +"""Tests for the simplified FlasherClient (no opendal dependency).""" + +from pathlib import Path + +import pytest + +from jumpstarter.client.flasher import _parse_path + + +class TestParsePath: + """Tests for _parse_path which routes local files vs HTTP URLs.""" + + def test_http_url(self): + local, url = _parse_path("http://example.com/image.qcow2") + assert local is None + assert url == "http://example.com/image.qcow2" + + def test_https_url(self): + local, url = _parse_path("https://download.fedoraproject.org/pub/fedora/image.qcow2") + assert local is None + assert url == "https://download.fedoraproject.org/pub/fedora/image.qcow2" + + def test_local_path_string(self, tmp_path): + test_file = tmp_path / "image.qcow2" + test_file.touch() + local, url = _parse_path(str(test_file)) + assert url is None + assert local == test_file.resolve() + + def test_local_path_object(self, tmp_path): + test_file = tmp_path / "image.qcow2" + test_file.touch() + local, url = _parse_path(test_file) + assert url is None + assert local == test_file.resolve() + + def test_relative_path(self): + local, url = _parse_path("relative/path/image.qcow2") + assert url is None + assert local is not None + assert local.is_absolute() + + def test_url_with_query_params(self): + test_url = "https://example.com/image.qcow2?token=abc&expires=123" + local, url = _parse_path(test_url) + assert local is None + assert url == test_url + + +class TestHttpUrlAdapter: + """Tests for _http_url_adapter which creates PresignedRequestResource for HTTP URLs.""" + + @pytest.mark.anyio + async def test_read_mode_produces_get_request(self): + from jumpstarter.client.flasher import _http_url_adapter + from jumpstarter.common.resources import PresignedRequestResource + + # _http_url_adapter is decorated with @blocking, but the underlying + # async generator can be tested directly via its __wrapped__ attribute + gen = _http_url_adapter.__wrapped__( + client=None, + url="https://example.com/firmware.bin", + mode="rb", + ) + result = await gen.__aenter__() + + # Should produce a serialized PresignedRequestResource with GET method + assert result["url"] == "https://example.com/firmware.bin" + assert result["method"] == "GET" + assert result["headers"] == {} + + await gen.__aexit__(None, None, None) + + @pytest.mark.anyio + async def test_write_mode_produces_put_request(self): + from jumpstarter.client.flasher import _http_url_adapter + + gen = _http_url_adapter.__wrapped__( + client=None, + url="https://example.com/dump.bin", + mode="wb", + ) + result = await gen.__aenter__() + + assert result["url"] == "https://example.com/dump.bin" + assert result["method"] == "PUT" + assert result["headers"] == {} + + await gen.__aexit__(None, None, None) + + +class TestFlasherClientRouting: + """Tests that FlasherClient routes HTTP URLs vs local paths correctly.""" + + def test_flash_single_routes_http_url(self): + """Verify that an HTTP URL goes through _http_url_adapter, not _local_file_adapter.""" + from unittest.mock import MagicMock, patch + + from jumpstarter.client.flasher import FlasherClient + + client = object.__new__(FlasherClient) + + mock_http = MagicMock() + mock_http.__enter__ = MagicMock(return_value="http_handle") + mock_http.__exit__ = MagicMock(return_value=False) + + mock_local = MagicMock() + + with ( + patch("jumpstarter.client.flasher._http_url_adapter", return_value=mock_http) as http_patch, + patch("jumpstarter.client.flasher._local_file_adapter", return_value=mock_local) as local_patch, + patch.object(client, "call", return_value=None) as call_mock, + ): + client._flash_single("https://example.com/image.bin", target=None, compression=None) + + http_patch.assert_called_once_with(client=client, url="https://example.com/image.bin", mode="rb") + local_patch.assert_not_called() + call_mock.assert_called_once_with("flash", "http_handle", None) + + def test_flash_single_routes_local_path(self, tmp_path): + """Verify that a local path goes through _local_file_adapter, not _http_url_adapter.""" + from unittest.mock import MagicMock, patch + + from jumpstarter.client.flasher import FlasherClient + + client = object.__new__(FlasherClient) + test_file = tmp_path / "image.bin" + test_file.touch() + + mock_local = MagicMock() + mock_local.__enter__ = MagicMock(return_value="local_handle") + mock_local.__exit__ = MagicMock(return_value=False) + + mock_http = MagicMock() + + with ( + patch("jumpstarter.client.flasher._http_url_adapter", return_value=mock_http) as http_patch, + patch("jumpstarter.client.flasher._local_file_adapter", return_value=mock_local) as local_patch, + patch.object(client, "call", return_value=None) as call_mock, + ): + client._flash_single(str(test_file), target=None, compression=None) + + local_patch.assert_called_once() + http_patch.assert_not_called() + call_mock.assert_called_once_with("flash", "local_handle", None) + + def test_dump_routes_http_url(self): + """Verify that dump with an HTTP URL goes through _http_url_adapter.""" + from unittest.mock import MagicMock, patch + + from jumpstarter.client.flasher import FlasherClient + + client = object.__new__(FlasherClient) + + mock_http = MagicMock() + mock_http.__enter__ = MagicMock(return_value="http_handle") + mock_http.__exit__ = MagicMock(return_value=False) + + with ( + patch("jumpstarter.client.flasher._http_url_adapter", return_value=mock_http) as http_patch, + patch("jumpstarter.client.flasher._local_file_adapter") as local_patch, + patch.object(client, "call", return_value=None) as call_mock, + ): + client.dump("https://example.com/dump.bin", target=None) + + http_patch.assert_called_once_with(client=client, url="https://example.com/dump.bin", mode="wb") + local_patch.assert_not_called() + call_mock.assert_called_once_with("dump", "http_handle", None) From 0a8e0d59852dbd37b86e5d56606a0bae5cf9f939 Mon Sep 17 00:00:00 2001 From: Ambient Code Bot Date: Fri, 10 Apr 2026 13:32:06 +0000 Subject: [PATCH 4/6] Fix lint: remove unused imports in flasher_test.py Remove unused `pathlib.Path` and `PresignedRequestResource` imports that were flagged by ruff (F401). Co-Authored-By: Claude Opus 4.6 --- python/packages/jumpstarter/jumpstarter/client/flasher_test.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/python/packages/jumpstarter/jumpstarter/client/flasher_test.py b/python/packages/jumpstarter/jumpstarter/client/flasher_test.py index d2b62cb7f..2464dc253 100644 --- a/python/packages/jumpstarter/jumpstarter/client/flasher_test.py +++ b/python/packages/jumpstarter/jumpstarter/client/flasher_test.py @@ -1,7 +1,5 @@ """Tests for the simplified FlasherClient (no opendal dependency).""" -from pathlib import Path - import pytest from jumpstarter.client.flasher import _parse_path @@ -53,7 +51,6 @@ class TestHttpUrlAdapter: @pytest.mark.anyio async def test_read_mode_produces_get_request(self): from jumpstarter.client.flasher import _http_url_adapter - from jumpstarter.common.resources import PresignedRequestResource # _http_url_adapter is decorated with @blocking, but the underlying # async generator can be tested directly via its __wrapped__ attribute From 69cbb0e15b388e3d63c84cdb52ecdf2c99e2592c Mon Sep 17 00:00:00 2001 From: Ambient Code Bot Date: Mon, 13 Apr 2026 20:35:04 +0000 Subject: [PATCH 5/6] Address review feedback: remove docstrings, fix aclose, add dump local test - Remove module-level docstrings from flasher.py, driver/flasher.py, and flasher_test.py to match project conventions (per raballew review) - Fix _AsyncIteratorStream.aclose() to propagate close to wrapped async generator, ensuring file handles are cleaned up on error paths - Add test_dump_routes_local_path to cover dump routing for local file paths Co-Authored-By: Claude Opus 4.6 --- .../jumpstarter/jumpstarter/client/flasher.py | 10 +------ .../jumpstarter/client/flasher_test.py | 26 +++++++++++++++++-- .../jumpstarter/jumpstarter/driver/flasher.py | 7 ----- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/python/packages/jumpstarter/jumpstarter/client/flasher.py b/python/packages/jumpstarter/jumpstarter/client/flasher.py index 7c101b7e6..1c536e0c1 100644 --- a/python/packages/jumpstarter/jumpstarter/client/flasher.py +++ b/python/packages/jumpstarter/jumpstarter/client/flasher.py @@ -1,11 +1,3 @@ -""" -Simplified FlasherClient that does not depend on opendal. - -For local files: streams via the existing resource_async mechanism using anyio. -For HTTP URLs: passes a PresignedRequestResource directly to the exporter, -which already handles presigned downloads via aiohttp. -""" - from __future__ import annotations from abc import ABCMeta, abstractmethod @@ -49,7 +41,7 @@ async def send_eof(self): pass async def aclose(self): - pass + await self.iterator.aclose() @property def extra_attributes(self) -> Mapping[Any, Callable[[], Any]]: diff --git a/python/packages/jumpstarter/jumpstarter/client/flasher_test.py b/python/packages/jumpstarter/jumpstarter/client/flasher_test.py index 2464dc253..566bb5dfc 100644 --- a/python/packages/jumpstarter/jumpstarter/client/flasher_test.py +++ b/python/packages/jumpstarter/jumpstarter/client/flasher_test.py @@ -1,5 +1,3 @@ -"""Tests for the simplified FlasherClient (no opendal dependency).""" - import pytest from jumpstarter.client.flasher import _parse_path @@ -163,3 +161,27 @@ def test_dump_routes_http_url(self): http_patch.assert_called_once_with(client=client, url="https://example.com/dump.bin", mode="wb") local_patch.assert_not_called() call_mock.assert_called_once_with("dump", "http_handle", None) + + def test_dump_routes_local_path(self, tmp_path): + """Verify that dump with a local path goes through _local_file_adapter.""" + from unittest.mock import MagicMock, patch + + from jumpstarter.client.flasher import FlasherClient + + client = object.__new__(FlasherClient) + test_file = tmp_path / "dump.bin" + + mock_local = MagicMock() + mock_local.__enter__ = MagicMock(return_value="local_handle") + mock_local.__exit__ = MagicMock(return_value=False) + + with ( + patch("jumpstarter.client.flasher._http_url_adapter") as http_patch, + patch("jumpstarter.client.flasher._local_file_adapter", return_value=mock_local) as local_patch, + patch.object(client, "call", return_value=None) as call_mock, + ): + client.dump(str(test_file), target=None) + + local_patch.assert_called_once() + http_patch.assert_not_called() + call_mock.assert_called_once_with("dump", "local_handle", None) diff --git a/python/packages/jumpstarter/jumpstarter/driver/flasher.py b/python/packages/jumpstarter/jumpstarter/driver/flasher.py index 47d655875..096314808 100644 --- a/python/packages/jumpstarter/jumpstarter/driver/flasher.py +++ b/python/packages/jumpstarter/jumpstarter/driver/flasher.py @@ -1,10 +1,3 @@ -""" -Common flasher interface for drivers that flash images to devices. - -This is a pure ABC with no external dependencies, providing a common interface -for flasher drivers across the jumpstarter ecosystem. -""" - from __future__ import annotations from abc import ABCMeta, abstractmethod From da111341d00241bd4eb131b4fc20107fc8e76134 Mon Sep 17 00:00:00 2001 From: Ambient Code Bot Date: Wed, 15 Apr 2026 11:35:28 +0000 Subject: [PATCH 6/6] Address review feedback: add stream tests, multi-target tests, compression warning - Add unit tests for _AsyncIteratorStream and _FileWriteObjectStream (receive/send/aclose lifecycle, error propagation, empty iterator) - Add tests for dict-based multi-target flash (happy path + ArgumentError) - Warn when compression parameter is used with HTTP URLs instead of silently ignoring it - Add tests for the compression warning behavior Co-Authored-By: Claude Opus 4.6 --- .../jumpstarter/jumpstarter/client/flasher.py | 11 + .../jumpstarter/client/flasher_test.py | 237 ++++++++++++++++++ 2 files changed, 248 insertions(+) diff --git a/python/packages/jumpstarter/jumpstarter/client/flasher.py b/python/packages/jumpstarter/jumpstarter/client/flasher.py index 1c536e0c1..7c79b183f 100644 --- a/python/packages/jumpstarter/jumpstarter/client/flasher.py +++ b/python/packages/jumpstarter/jumpstarter/client/flasher.py @@ -1,5 +1,6 @@ from __future__ import annotations +import warnings from abc import ABCMeta, abstractmethod from contextlib import asynccontextmanager from dataclasses import dataclass, field @@ -232,6 +233,11 @@ def _flash_single( local_path, url = _parse_path(image) if url is not None: + if compression is not None: + warnings.warn( + "compression parameter is ignored for HTTP URLs", + stacklevel=2, + ) # HTTP URL: pass as presigned request for exporter-side download with _http_url_adapter(client=self, url=url, mode="rb") as handle: return self.call("flash", handle, target) @@ -271,6 +277,11 @@ def dump( local_path, url = _parse_path(path) if url is not None: + if compression is not None: + warnings.warn( + "compression parameter is ignored for HTTP URLs", + stacklevel=2, + ) with _http_url_adapter(client=self, url=url, mode="wb") as handle: return self.call("dump", handle, target) else: diff --git a/python/packages/jumpstarter/jumpstarter/client/flasher_test.py b/python/packages/jumpstarter/jumpstarter/client/flasher_test.py index 566bb5dfc..c306b6108 100644 --- a/python/packages/jumpstarter/jumpstarter/client/flasher_test.py +++ b/python/packages/jumpstarter/jumpstarter/client/flasher_test.py @@ -1,3 +1,5 @@ +import warnings + import pytest from jumpstarter.client.flasher import _parse_path @@ -84,6 +86,138 @@ async def test_write_mode_produces_put_request(self): await gen.__aexit__(None, None, None) +class TestAsyncIteratorStream: + """Tests for _AsyncIteratorStream receive/send/aclose lifecycle.""" + + @pytest.mark.anyio + async def test_receive_yields_chunks(self): + from anyio import EndOfStream + + from jumpstarter.client.flasher import _AsyncIteratorStream + + async def gen(): + yield b"chunk1" + yield b"chunk2" + + stream = _AsyncIteratorStream(iterator=gen(), total=12) + assert await stream.receive() == b"chunk1" + assert await stream.receive() == b"chunk2" + with pytest.raises(EndOfStream): + await stream.receive() + + @pytest.mark.anyio + async def test_send_raises_broken_resource(self): + from anyio import BrokenResourceError + + from jumpstarter.client.flasher import _AsyncIteratorStream + + async def gen(): + yield b"data" + + stream = _AsyncIteratorStream(iterator=gen()) + with pytest.raises(BrokenResourceError): + await stream.send(b"data") + + @pytest.mark.anyio + async def test_aclose_propagates_to_generator(self): + from jumpstarter.client.flasher import _AsyncIteratorStream + + closed = False + + async def gen(): + nonlocal closed + try: + yield b"data" + yield b"more" + finally: + closed = True + + stream = _AsyncIteratorStream(iterator=gen()) + await stream.receive() + await stream.aclose() + assert closed + + @pytest.mark.anyio + async def test_extra_attributes_with_total(self): + from jumpstarter.client.flasher import _AsyncIteratorStream + from jumpstarter.streams.progress import ProgressAttribute + + async def gen(): + yield b"data" + + stream = _AsyncIteratorStream(iterator=gen(), total=100) + attrs = stream.extra_attributes + assert ProgressAttribute.total in attrs + assert attrs[ProgressAttribute.total]() == 100.0 + + @pytest.mark.anyio + async def test_extra_attributes_without_total(self): + from jumpstarter.client.flasher import _AsyncIteratorStream + + async def gen(): + yield b"data" + + stream = _AsyncIteratorStream(iterator=gen(), total=None) + assert stream.extra_attributes == {} + + @pytest.mark.anyio + async def test_receive_on_empty_iterator(self): + from anyio import EndOfStream + + from jumpstarter.client.flasher import _AsyncIteratorStream + + async def gen(): + return + yield # noqa: RET504 + + stream = _AsyncIteratorStream(iterator=gen()) + with pytest.raises(EndOfStream): + await stream.receive() + + +class TestFileWriteObjectStream: + """Tests for _FileWriteObjectStream send/aclose lifecycle.""" + + @pytest.mark.anyio + async def test_write_and_read_back(self, tmp_path): + from jumpstarter.client.flasher import _FileWriteObjectStream + + out = tmp_path / "output.bin" + stream = _FileWriteObjectStream(path=out) + await stream.send(b"hello ") + await stream.send(b"world") + await stream.send_eof() + assert out.read_bytes() == b"hello world" + + @pytest.mark.anyio + async def test_receive_raises_end_of_stream(self, tmp_path): + from anyio import EndOfStream + + from jumpstarter.client.flasher import _FileWriteObjectStream + + stream = _FileWriteObjectStream(path=tmp_path / "out.bin") + with pytest.raises(EndOfStream): + await stream.receive() + + @pytest.mark.anyio + async def test_aclose_without_open(self, tmp_path): + from jumpstarter.client.flasher import _FileWriteObjectStream + + stream = _FileWriteObjectStream(path=tmp_path / "out.bin") + await stream.aclose() + + @pytest.mark.anyio + async def test_aclose_closes_file(self, tmp_path): + from jumpstarter.client.flasher import _FileWriteObjectStream + + out = tmp_path / "output.bin" + stream = _FileWriteObjectStream(path=out) + await stream.send(b"data") + await stream.aclose() + assert out.read_bytes() == b"data" + assert stream._file is None + + class TestFlasherClientRouting: """Tests that FlasherClient routes HTTP URLs vs local paths correctly.""" @@ -185,3 +319,106 @@ def test_dump_routes_local_path(self, tmp_path): local_patch.assert_called_once() http_patch.assert_not_called() call_mock.assert_called_once_with("dump", "local_handle", None) + + +class TestFlasherClientMultiTarget: + """Tests for dict-based multi-target flash.""" + + def test_flash_dict_calls_flash_single_per_entry(self): + from unittest.mock import MagicMock, patch + + from jumpstarter.client.flasher import FlasherClient + + client = object.__new__(FlasherClient) + + mock_http = MagicMock() + mock_http.__enter__ = MagicMock(return_value="handle") + mock_http.__exit__ = MagicMock(return_value=False) + + with ( + patch("jumpstarter.client.flasher._http_url_adapter", return_value=mock_http), + patch.object(client, "call", return_value="ok") as call_mock, + ): + results = client.flash( + {"boot": "https://example.com/boot.bin", "root": "https://example.com/root.bin"}, + compression=None, + ) + assert results == {"boot": "ok", "root": "ok"} + assert call_mock.call_count == 2 + + def test_flash_dict_with_target_raises_argument_error(self): + from jumpstarter.client.flasher import FlasherClient + from jumpstarter.common.exceptions import ArgumentError + + client = object.__new__(FlasherClient) + + with pytest.raises(ArgumentError, match="'target' parameter is not valid"): + client.flash({"boot": "/tmp/boot.bin"}, target="some_target") + + +class TestCompressionWarning: + """Tests that compression parameter warns when used with HTTP URLs.""" + + def test_flash_http_with_compression_warns(self): + from unittest.mock import MagicMock, patch + + from jumpstarter.client.flasher import FlasherClient + + client = object.__new__(FlasherClient) + + mock_http = MagicMock() + mock_http.__enter__ = MagicMock(return_value="handle") + mock_http.__exit__ = MagicMock(return_value=False) + + with ( + patch("jumpstarter.client.flasher._http_url_adapter", return_value=mock_http), + patch.object(client, "call", return_value=None), + ): + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + client._flash_single("https://example.com/image.bin", target=None, compression="zstd") + assert len(w) == 1 + assert "compression parameter is ignored" in str(w[0].message) + + def test_flash_local_with_compression_no_warning(self, tmp_path): + from unittest.mock import MagicMock, patch + + from jumpstarter.client.flasher import FlasherClient + + client = object.__new__(FlasherClient) + test_file = tmp_path / "image.bin" + test_file.touch() + + mock_local = MagicMock() + mock_local.__enter__ = MagicMock(return_value="handle") + mock_local.__exit__ = MagicMock(return_value=False) + + with ( + patch("jumpstarter.client.flasher._local_file_adapter", return_value=mock_local), + patch.object(client, "call", return_value=None), + ): + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + client._flash_single(str(test_file), target=None, compression="zstd") + assert len(w) == 0 + + def test_dump_http_with_compression_warns(self): + from unittest.mock import MagicMock, patch + + from jumpstarter.client.flasher import FlasherClient + + client = object.__new__(FlasherClient) + + mock_http = MagicMock() + mock_http.__enter__ = MagicMock(return_value="handle") + mock_http.__exit__ = MagicMock(return_value=False) + + with ( + patch("jumpstarter.client.flasher._http_url_adapter", return_value=mock_http), + patch.object(client, "call", return_value=None), + ): + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + client.dump("https://example.com/dump.bin", target=None, compression="zstd") + assert len(w) == 1 + assert "compression parameter is ignored" in str(w[0].message)