This repository was archived by the owner on Feb 22, 2026. It is now read-only.
fix(storage): ensure parent directory exists before opening ZarrStream#65
Merged
Conversation
acquire-zarr requires the parent directory of the store URI to exist before ZarrStream is created. _build_writer in redsun creates the base directory at container-build time, but on Windows the file:// URI round-trip (Path.as_uri() -> ZarrStream) can fail to resolve to the same filesystem path that mkdir created, causing: ZarrStream_s: Parent path '...' does not exist or is not a directory Fix: add ZarrWriter._ensure_parent(store_uri) called at the top of kickoff(), just before ZarrStream is opened. It parses the file:// URI back to a filesystem path (handling the Windows /C:/... leading slash), and mkdir -p's the parent. Non-file:// URIs (s3://, etc.) are skipped unconditionally.
The previous approach parsed the store URI back to a filesystem path in _ensure_parent(), but the Windows URI round-trip (Path.as_uri() -> urlparse -> path_str) produced '/C:/Users/...' which Path() on Windows interpreted as a rooted path without a drive letter, causing: OSError: [WinError 123] The filename, directory name, or volume label syntax is incorrect: '\C:\Users\...' Simpler fix: ZarrWriter now accepts base_dir: Path directly at construction time. kickoff() calls base_dir.mkdir(parents=True, exist_ok=True) — a plain Path operation with no URI parsing. _build_writer in redsun already has base_dir as a Path, so it passes it straight through. No platform-specific logic needed anywhere.
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
…lity These features were speculative design notes that are not implemented. Keeping them in the docs creates false expectations. Simplified to describe only what actually exists: local file:// stores via ZarrWriter.
Previously _HasStorage and _HasStorageMeta lived as private implementation details in redsun.containers.container, with two TODO comments asking for them to be moved here. Changes: - HasStorageMeta: metaclass that overrides __instancecheck__ to walk the MRO looking for a StorageDescriptor, so isinstance(device, HasStorage) is True only for devices that have genuinely opted in to storage - HasStorage: public runtime_checkable Protocol with storage: StorageProxy (not StorageProxy | None), so after an isinstance check mypy narrows the type without requiring a if device.storage is not None guard - Both exported from sunflare.storage.__init__ and documented
…uisition/sunflare into fix/zarr-parent-mkdir
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
acquire-zarr requires the parent directory of the store URI to exist before ZarrStream is created. _build_writer in redsun creates the base directory at container-build time, but on Windows the file:// URI round-trip (Path.as_uri() -> ZarrStream) can fail to resolve to the same filesystem path that mkdir created, causing:
ZarrStream_s: Parent path '...' does not exist or is not a directory
Fix: add ZarrWriter._ensure_parent(store_uri) called at the top of kickoff(), just before ZarrStream is opened. It parses the file:// URI back to a filesystem path (handling the Windows /C:/... leading slash), and mkdir -p's the parent. Non-file:// URIs (s3://, etc.) are skipped unconditionally.