feat: versioned DatasetStorage for reading/writing datasets across protocol versions by koenvanderveen · Pull Request #9443 · OpenMined/PySyft

koenvanderveen · 2026-07-15T12:56:52Z

Summary

Mirrors the syft-job migration/storage architecture (PR #9434) in syft-datasets, adding a versioned read/write layer so datasets can evolve their on-disk format while staying compatible with older peers.

DatasetStorage — the single choke point for all dataset filesystem IO. Reads select a ProtocolCodec by the on-disk protocol version and upgrade the object to the latest version in memory; writes downgrade to the target protocol and let the codec persist it. Nests a MigrationRegistry, peer_schemas, and codecs.
v<n> path segment — new datasets can live under public/syft_datasets/v<n>/<name>/ (and private/…). Protocol 0 = the last release (syft-client 0.1.117 / syft-dataset 0.1.20): flat layout, no v<n>, no canonical_name/version fields — read and written back byte-compatibly.
Versioned models — Dataset / PrivateDatasetConfig are now MigratableObjects under models/<object>/v1.py. Object schemas are unchanged since 0.1.117, so only v1 exists; the protocol bump is purely a path-layout + identity-field change. dataset.py is kept as a re-export shim.
DatasetRef — path-derived identity (owner, name, protocol_version); the dataset name comes from the path, not the spoofable file.
Broadcast write — a public dataset is a single copy read by the whole audience (unlike jobs, which target one peer), so the write protocol(s) come from the audience's versions (target_protocol_versions_for_peers), defaulting to the widest-compatible (oldest) protocol when no peers are known. peer_schemas is injectable but not yet populated by syft-client, so today datasets are written in protocol 0 and the v1 layout ships dormant (writing v1 now would hide datasets from 0.1.117 peers).
SyftDatasetManager routes all reads/writes through DatasetStorage and supports multi-version write (one on-disk copy per protocol version in the audience); get_all dedupes copies across layouts, preferring the newest protocol.
Release tooling — scripts/export_release_artifact.py + generate_release_fixture.py; the protocol-0 / 0.1.20 artifacts and fixtures are derived as if that release had emitted them.
Tests mirror syft-job/tests/migrations (unit + p2p), including a byte-exact cross-release round-trip over hand-generated fixtures and a multi-version-write test. CI (unit-tests.yml dataset-tests job) and justfile (test-unit-datasets) wired.

Scope

Package core only, per discussion. syft-client wiring (advertising each peer's dataset protocol version and writing in it at create/share) is a follow-up — syft-client currently exchanges only a single client-version string, not per-package schemas.

Testing

uv run pytest packages/syft-datasets/tests → 40 passed
just test-unit-fast → 406 passed
tests/unit dataset + sync_manager + delete suites → green
pre-commit run --all-files → clean

…otocol versions Mirror the syft-job migration/storage architecture in syft-datasets: - DatasetStorage: single choke point for all dataset filesystem IO. Reads select a ProtocolCodec by on-disk protocol version and upgrade to the latest object version in memory; writes downgrade to the target protocol and the codec persists it. Nests a MigrationRegistry, peer_schemas, and codecs. - On-disk protocol version: new datasets can live under a v<n> path segment (public/syft_datasets/v<n>/<name>/). Protocol 0 = the last release (syft-client 0.1.117 / syft-dataset 0.1.20): flat layout, no v<n>, no canonical_name/version fields; read + written back byte-compatibly. - Versioned models: Dataset / PrivateDatasetConfig are now MigratableObjects under models/<object>/v1.py. Object schemas are unchanged since 0.1.117, so only v1 exists; the protocol bump is purely path-layout + identity fields. - DatasetRef: path-derived identity (owner, name, protocol_version). - Broadcast write: a public dataset is one copy read by the whole audience, so the write protocol(s) come from the audience's versions (target_protocol_versions_for_peers), defaulting to the widest-compatible (oldest) protocol when no peers are known. peer_schemas is injectable but not yet populated by syft-client, so today datasets are written in protocol 0 and the v1 layout ships dormant. - SyftDatasetManager routes all reads/writes through DatasetStorage and supports multi-version write (one copy per protocol version in the audience); get_all dedupes copies across layouts preferring the newest protocol. - migrations/history + release scripts (export_release_artifact.py, generate_release_fixture.py); protocol-0/0.1.20 artifacts and fixtures derived as if that release had emitted them. - Tests mirror syft-job/tests/migrations (unit + p2p, incl. byte-exact cross-release round-trip and a multi-version-write test); CI + justfile wired. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

The enclave inference example computed private dataset dirs using the default protocol version (v1 segment), but datasets shared with 'any' are written at the widest-compatible protocol (0, no segment). This mismatch broke the log-sink path and the ensure_logs_dataset idempotency check. Resolve the actual on-disk ref via DatasetStorage, falling back to the widest-compatible protocol for datasets not yet synced.

koenvanderveen · 2026-07-20T09:07:20Z

 [project]
 name = "syft-dataset"
-version = "0.1.20"
+version = "0.1.21"


is this necessary?

koenvanderveen · 2026-07-20T09:12:16Z

+)
+
+
+def _seed_syftbox(syftbox: Path, src_dir: Path, protocol_version: str) -> None:


double check this script

package-artifacts/ and protocols/ hold released history only. Only 0.1.20 (protocol 0) shipped to PyPI, so the frozen 0.1.21/protocol-1 artifacts were premature. The unreleased protocol 1 already lives in the live registry and will be frozen by export_release_artifact.py at release time (mirrors syft-job). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

koenvanderveen · 2026-07-20T10:47:52Z

    def public_dir(self) -> Path:
        return self.syftbox_folder / self.email / "public"

+    def public_datasets_root_for_datasite(self, datasite: str) -> Path:


it feels like this should be part of the codec

koenvanderveen · 2026-07-20T10:49:10Z

-from syft_permissions.spec.ruleset import PERMISSION_FILE_NAME
-from typing_extensions import Self
-from uuid import UUID, uuid4
+"""Backwards-compatible re-exports.


Do we still need this file at all? Where do we use these "historical import paths"

koenvanderveen · 2026-07-20T10:53:05Z

        tags = tags or []

-        mock_url = self.syftbox_config.get_mock_url_for_my_dataset(dataset_name=name)
+        target_versions = self._target_protocol_versions(users)


we should probably by default always write in the current version

koenvanderveen · 2026-07-20T10:54:49Z

-        private_dir = self.syftbox_config.private_dir_for_my_dataset(
-            dataset_name=dataset_name
-        )
+    def _prepare_private_data(self, private_dir: Path, src_path: Path) -> list[Path]:


private_dir should be called private_target_dir

koenvanderveen · 2026-07-20T10:55:33Z

-            dataset_name=dataset_name
-        )
+    def _prepare_readme(
+        self, target_mock_dir: Path, src_file: Path | None


target_mock_dir should be called target_dir here

koenvanderveen · 2026-07-20T10:56:51Z

-        target_mock_dir = self.syftbox_config.get_my_mock_dataset_dir(
-            dataset_name=dataset_name
-        )
+    def _prepare_mock_data(self, target_mock_dir: Path, src_path: Path) -> list[Path]:


all these _prepare_<x> methods should have a ref as input, compute the target path and do the copying

koenvanderveen · 2026-07-20T11:00:04Z


        return dataset_manager_repr_html(self.get_all())

+    def _datasites_to_check(self, datasite: str | None) -> list[str]:


this method has a weird name. Perhaps it should have no args and we should do something like datasites = datasite if datasite is not None else self._all_syftbox_datasites() when we call it

koenvanderveen · 2026-07-20T11:17:20Z

this whole folder should probably not exist because this version has not been released yet

koenvanderveen · 2026-07-20T11:35:24Z

@@ -166,86 +136,106 @@ def create(
        users: list[str] | str | None = None,
        # copy_private_data: bool = True, # TODO


perhaps we should also have the posibility to pass in protocol verisons here in case we want to override something

koenvanderveen · 2026-07-20T11:39:33Z

+            codec.read(path, "PrivateDatasetConfig"), "PrivateDatasetConfig"
+        )
+
+    def write_dataset(self, ref: DatasetRef, dataset: Dataset) -> Path:


probably for all the writes of datasets, we first want to see which versions are already on disk. If we have multiple versions on disk already we need to make sure that they are in sync. So we first need a step that checks the existing protocols on disk, then finds the codecs for those protocols (and add the ref protocol) and then write for all codecs

koenvanderveen · 2026-07-20T11:41:58Z

+        peer_emails = None if users in (None, SHARE_WITH_ANY) else list(users)
+        return self.storage.target_protocol_versions_for_peers(peer_emails)
+
+    def _create_one_version(


I think the responsibility of this should probably lie with the storage layer

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

koenvanderveen · 2026-07-20T12:11:00Z

+from syft_datasets.models import DatasetV1
+
+
+def test_0_1_20_artifact_file_loads():


this should be more generic, it should just load every package artifact that was released

koenvanderveen · 2026-07-20T12:11:27Z

+            assert schema.current_schema(canonical_name)
+
+
+def test_protocol_0_released_protocol_loads():


we can remove this, we already have the one that loads all of them

pjwerneck

A few issues found and commented on.

Overall, the high-level design is in line with #9434, but the fact peer_schemas is still not properly wired is a bit concerning. In production, everything still resolves to v0. I'd be more comfortable if v1 and the whole protocol mechanics was fully integrated and tested end-to-end before wiring it to the rest of syft-client.

pjwerneck · 2026-07-20T15:06:27Z

+        for ref in best_ref.values():
+            try:
+                all_datasets.append(self.storage.read_dataset(ref))
+            except Exception:


Blanket Exception catch with no logging. This will silently ignore errors due to unreadable datasets in read_dataset. Not minor issue, since migration failures or schema inconsistencies can make the dataset vanish with no logs.

pjwerneck · 2026-07-20T15:19:21Z

+        dataset_uid = uuid4()
+
+        created: dict[str, Dataset] = {}
+        for protocol_version in sorted(target_versions, key=int):


This can create a dataset in several protocol versions at once, but delete() uses get() which uses store.find_dataset_ref(), which returns only a single dataset, with the newest protocol. It's going to leave older copies. Is that intended? If not, delete() should iterate all versions too.

pjwerneck · 2026-07-20T15:24:50Z

+
+    @property
+    def owner(self) -> str:
+        return self.mock_url.host


Shouldn't the canonical owner come from the path?

pjwerneck · 2026-07-20T15:27:54Z

-        private_path: Path = to_path(private_path)
-        readme_path: Path | None = to_path(readme_path) if readme_path else None
+        # Reject names that collide with a protocol-version directory (v1, v2, ...).
+        # (The stricter _validate_dataset_name regex is intentionally not enforced


In that case, SyftDatasetManager._validate_dataset_name is dead code and should be removed.

pjwerneck · 2026-07-20T16:07:23Z

+        )
+
+    def private_dataset_dir(self, ref: DatasetRef) -> Path:
+        return self.config.get_private_dataset_dir(


Both codecs (v0 and v1) are deferring the public and private paths to config. Wasn't the point of the refactoring to have the codec own disk layout?

pjwerneck · 2026-07-20T16:11:01Z

-            config.model_dump(mode="json"), indent=2, sort_keys=False
-        ).encode()
+        data = config.disk_dict()
+        if ref.protocol_version == "0":


This should be handled by the codec, not by the manager. The manager should never make internal decisions based on protocol version.

pjwerneck · 2026-07-20T16:16:48Z

+        data = codec.read(codec.metadata_path(ref), "Dataset")
+        dataset = self._upgrade(data, "Dataset")
+        # Name comes from the path (the dataset dir), never a spoofable file.
+        dataset.name = ref.name


This is the counterpart to the comment on owner property in models/dataset/v1.py:69. If we assign the name from ref.name here, why not the owner too?

pjwerneck · 2026-07-20T16:19:22Z

 from pydantic import BaseModel, Field
 from pathlib import Path
 from .url import SyftBoxURL
+from .migrations.registry import DATASET_PROTOCOL_VERSION


The config.py now has a protocol_version parameter everywhere, which defaults to this imported DATASET_PROTOCOL_VERSION,the current protocol, but the storage defaults to the widest. Is that intended?

github-actions Bot added feature pkg:syft-dataset labels Jul 15, 2026

koenvanderveen commented Jul 20, 2026

View reviewed changes

test: rename dataset test helpers for clarity

53eb6e6

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

koenvanderveen commented Jul 20, 2026

View reviewed changes

pjwerneck reviewed Jul 20, 2026

View reviewed changes

		)


		def _seed_syftbox(syftbox: Path, src_dir: Path, protocol_version: str) -> None:


		return dataset_manager_repr_html(self.get_all())

		def _datasites_to_check(self, datasite: str \| None) -> list[str]:

		@@ -166,86 +136,106 @@ def create(
		users: list[str] \| str \| None = None,
		# copy_private_data: bool = True, # TODO

		from syft_datasets.models import DatasetV1


		def test_0_1_20_artifact_file_loads():

		assert schema.current_schema(canonical_name)


		def test_protocol_0_released_protocol_loads():

Uh oh!

Uh oh!

Conversation

koenvanderveen commented Jul 15, 2026

Summary

Scope

Testing

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

koenvanderveen Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pjwerneck left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

koenvanderveen Jul 20, 2026 •

edited

Loading