fix: gaps in versioned objects for dataset objects

[pjwerneck]

Summary

Two places still handled the serialization of private_metadata.yaml manually instead of going through
the protocol codec, so both ignored the dataset's protocol version. Both now call the storage layer.

Changes

• The peer's copy of private_metadata.yaml is now serialized through the codec.
• Dataset.private_config goes through storage.

Testing

Asana task

https://app.asana.com/1/1185126988600652/project/1210542925864934/task/1216207166877921?focus=true

[pjwerneck]

PR 9510 — fix: gaps in versioned objects for dataset objects

OpenMined/PySyft#9510 · pjwerneck · pjwerneck/migrations-fix-versioned-dataset-objects → dev · +180/-34 across 8 files · OPEN

Two code paths serialized or deserialized private_metadata.yaml by hand with yaml, so neither
respected the protocol version of the dataset copy they were working on. The bytes shipped to an
enclave always carried the newest object shape, and Dataset.private_config always returned a
version-1 object. Both now go through DatasetStorage, which downgrades or upgrades first. To make
that possible, write() moved onto the base codec and each codec now supplies only the dict to
serialize.

Read time ~4 min. Not read: nothing — no lock, generated or notebook files in this PR.

---

• 1. Flows

  • 1.1 Shipping a dataset's private files to an enclave (changed)

    • When a data owner calls client.py: RDSClient.share_private_dataset(), it resolves the
      protocol version the enclave can read and calls
      dataset_manager.py: SyftDatasetManager.get_private_dataset_files(), which walks the
      private dataset folder and, for private_metadata.yaml only, substitutes the bytes from
      SyftDatasetManager._private_config_without_data_dir().
    • Inside that method we previously called config.disk_dict() and yaml.safe_dump()
      directly, so the peer received the newest object shape — canonical_name and version
      keys included — no matter which protocol layout the copy on disk used. Now we call the new
      DatasetStorage.wire_bytes().
    • DatasetStorage.wire_bytes() downgrades the config to the schema for the ref's protocol
      version, then hands it to that version's codec to shape. A copy in the v0 layout therefore
      ships with the identity keys stripped, which is what a pre-versioning (<= 0.1.20) peer
      expects to parse.
    • Decision: the peer bytes could have kept their own serializer, since they go to a peer and
      not to a path. They share the codec instead, so there is one definition of each layout's
      on-disk shape and the two cannot drift apart.

  • 1.2 Reading a dataset's own private config (changed)

    • When code reads Dataset.private_config, we previously loaded the YAML file with
      yaml.safe_load() inside the property, filled in canonical_name and version defaults,
      and constructed PrivateDatasetConfigV1 by name — so the property returned a version-1
      object forever, even once a later release registers a version 2.
    • Now the property builds a DatasetStorage from the dataset's own syftbox_config and
      calls DatasetStorage.read_private_config() with the dataset's _ref. That reads the file
      through the ref's codec and migrates the object up to the newest version in
      dataset_registry.
    • The property is a cached_property, so the storage object and the migration happen once
      per Dataset instance.

  • 1.3 A codec persisting an object to disk (changed)

    • When DatasetStorage writes dataset metadata or a private config, it calls
      DatasetStorage._write(), which downgrades the object and then calls
      ProtocolCodec.write(). Each codec previously implemented write() end to end: build the
      dict, create the parent folder, yaml.safe_dump() into the path.
    • Now ProtocolCodec.write() and ProtocolCodec.dumps() are concrete methods on the base
      class, and each codec implements only _data_for_disk(). The same shaping now serves a
      write to a path and the peer bytes of Flow 1.1.
    • Decision: _data_for_disk() stays abstract rather than defaulting to obj.disk_dict().
      A default would let a future codec inherit the v1 shape silently; abstract forces each new
      codec to state the layout it speaks.

• 2. What is new — additions only

  • NEW method DatasetStorage.wire_bytes() — packages/syft-datasets/src/syft_datasets/dataset_storage.py:532
    — returns the bytes to send to a peer for one object under a ref's protocol version, by
    downgrading and then calling the codec's dumps(). Called by
    SyftDatasetManager._private_config_without_data_dir(). (Flow 1.1)
  • NEW method DatasetStorage._downgrade() — packages/syft-datasets/src/syft_datasets/dataset_storage.py:547
    — looks up the schema for a ref's protocol version and calls
    MigrationService.migrate_to_schema(). Lifted out of DatasetStorage._write() so
    wire_bytes() can reuse it. (Flows 1.1, 1.3)
  • NEW methods on ProtocolCodec — packages/syft-datasets/src/syft_datasets/protocolcodecs/base.py
    — dumps() at :64 returns the YAML string for an object in this codec's format;
    write() at :68 creates the parent folder and writes that string; _data_for_disk() at :55
    is the new abstract hook each codec fills in. (Flow 1.3)
  • NEW method DatasetV1._require_owner() — packages/syft-datasets/src/syft_datasets/models/dataset/v1.py:101
    — raises ValueError when syftbox_config.email is not the dataset's owner, naming the
    thing being reached for in the message. (Flow 1.2)

• 3. Changes — everything changed or deleted except tests, grouped by theme

  • A — Peer bytes and disk writes share one downgrade

    • A1 the enclave's copy of private_metadata.yaml (Flow 1.1) — when
      SyftDatasetManager.get_private_dataset_files() needs the config bytes, we previously
      serialized the latest object shape with yaml.safe_dump() regardless of the copy's
      layout; now we call DatasetStorage.wire_bytes() so the bytes match the layout the copy
      is stored in. packages/syft-datasets/src/syft_datasets/dataset_manager.py: SyftDatasetManager._private_config_without_data_dir()
    • A2 the downgrade before a write (Flow 1.3) — DatasetStorage._write() previously
      looked up the protocol schema and called migrate_to_schema() in its own body; it now
      calls DatasetStorage._downgrade(), the same helper wire_bytes() uses.
      packages/syft-datasets/src/syft_datasets/dataset_storage.py
    • Mechanical: the yaml import is gone from dataset_manager.py, which no longer
      serializes anything itself.

  • B — Codecs describe their layout, the base class writes it

    • B1 the flat pre-versioning layout (Flow 1.3) — ProtocolCodecV0.write() is replaced
      by ProtocolCodecV0._data_for_disk(), which still calls obj.disk_dict() and pops
      canonical_name and version to byte-match the <= 0.1.20 format, but returns the dict
      instead of writing it.
      packages/syft-datasets/src/syft_datasets/protocolcodecs/v0.py
    • B2 the versioned nested layout (Flow 1.3) — ProtocolCodecV1.write() is replaced by
      ProtocolCodecV1._data_for_disk(), which returns obj.disk_dict() unchanged, because
      this layout keeps the identity fields on disk.
      packages/syft-datasets/src/syft_datasets/protocolcodecs/v1.py
    • Mechanical: protocolcodecs/base.py gains the yaml import that v0.py and v1.py no
      longer need for dumping. Both still import yaml for their read() methods.

  • C — Dataset stops parsing its own private config

    • C1 Dataset.private_config (Flow 1.2) — the property previously read and parsed
      private_metadata.yaml and built a PrivateDatasetConfigV1; it now delegates to
      DatasetStorage.read_private_config(), which upgrades to the latest version. The
      FileNotFoundError it used to raise itself is replaced by the
      PrivateConfigNotFoundError that storage raises — a FileNotFoundError subclass, so an
      existing caller catching FileNotFoundError still catches it.
      packages/syft-datasets/src/syft_datasets/models/dataset/v1.py: DatasetV1.private_config
    • C2 the owner check — DatasetV1.private_config_path and
      DatasetV1._private_metadata_dir each held their own copy of the
      syftbox_config.email != owner check and its raise ValueError; both now call
      DatasetV1._require_owner(), which takes the noun for the message. The two messages read
      the same as before, "private config" and "private data" respectively.
    • C3 the declared type — models/dataset/v1.py previously imported
      PrivateDatasetConfigV1 from ..private_dataset_config.v1; it now imports the
      PrivateDatasetConfig alias from ..private_dataset_config, so the annotation on
      private_config tracks whichever version is current rather than pinning version 1.
    • C4 an import inside the property — DatasetV1.private_config imports
      DatasetStorage in its body, with a comment saying why: dataset_storage imports this
      module, so a module-level import would be circular.

• 4. Tests — all pass: 23 tests in packages/syft-datasets/tests/migrations/p2p/

  • NEW in packages/syft-datasets/tests/migrations/p2p/test_protocol_codecs.py — 2 tests
    and 1 fixture, driving DatasetStorage against a tmp_path SyftBox folder
    • test_wire_bytes_match_write_and_strip_private_config_identity_on_v0() — writing one
      PrivateDatasetConfig under a v0 ref and a v1 ref, DatasetStorage.wire_bytes() returns
      exactly the bytes each write_private_config() put on disk; the v0 file holds only uid
      and data_dir, and the v1 file holds canonical_name and version as well.
    • fixture private_config_v2 — registers a throwaway PrivateDatasetConfigV2 and a 1 → 2
      migration in the global dataset_registry, then pops both back out on teardown, since the
      registry has no deregister call and a leaked version 2 would be visible to every later
      test.
    • test_dataset_private_config_upgrades_a_v1_file_to_the_latest_version() — given a
      hand-written version-1 private_metadata.yaml and a registered version 2,
      Dataset.private_config returns a PrivateDatasetConfigV2 whose uid still matches the
      file, so the migration ran and did not discard the file's data.
    • Decision: the second test asserts the uid explicitly, because a migration that returned
      a fresh default V2 would satisfy the version assertion on its own.
  • NEW in packages/syft-datasets/tests/migrations/p2p/test_current_protocol.py — 1 test
    and 1 helper, going through SyftDatasetManager end to end
    • test_private_files_wire_private_metadata_in_protocol_format() — for a dataset created in
      the v0 layout, the private_metadata.yaml returned by
      SyftDatasetManager.get_private_dataset_files() carries uid but no canonical_name or
      version; for one created with protocol_versions=["1"] it carries both. In both cases
      data_dir is not an absolute path, so the owner's local path does not leak to the peer.
    • helper test_current_protocol.py: _wire_private_metadata() — pulls the single
      private_metadata.yaml entry out of the returned file dict and parses it, asserting there
      is exactly one.

• 5. Code standards — new code only

  • packages/syft-datasets/tests/migrations/p2p/test_protocol_codecs.py: test_wire_bytes_match_write_and_strip_private_config_identity_on_v0()
    — the name says on_v0 but the test also asserts the v1 ref keeps its identity keys, and it
    joins two claims with and; test_wire_bytes_match_written_file_per_protocol() says what it
    checks.
  • packages/syft-datasets/tests/migrations/p2p/test_protocol_codecs.py: private_config_v2()
    — imports PrivateDatasetConfigV1 inside the fixture body, though the file already imports
    from syft_datasets.models at the top and there is no circular import to break.