feat: Init RDS package by rasswanth-s · Pull Request #9438 · OpenMined/PySyft

rasswanth-s · 2026-07-08T13:04:09Z

Summary

Changes

Testing

Pending

Asana task

(https://app.asana.com/1/1185126988600652/project/1210542925864934/task/1216142561490291)

PR naming convention

Your PR title must follow this format or merging will be blocked:

type: short description in lowercase

Example titles:

feat: add retry logic to job approval
fix: handle timeout in notification sender
docs: update syft-bg README
chore: bump dependencies
refactor: split init flow into helpers
test: add criteria validation tests
ci: add release train workflow

Allowed types:

Type	When to use	Example
`feat`	New feature or capability	`feat: add DS rejection emails`
`fix`	Bug fix	`fix: handle empty peer list`
`docs`	Documentation only	`docs: update syft-bg README`
`chore`	Maintenance, deps, config	`chore: bump dependencies`
`refactor`	Code restructuring (no behavior change)	`refactor: split init flow`
`test`	Adding or updating tests	`test: add approval criteria tests`
`ci`	CI/CD workflow changes	`ci: add release train workflow`
`perf`	Performance improvement	`perf: cache Drive API responses`
`build`	Build system or dependency changes	`build: pin syft-bg>=0.2.0`

Just edit the PR title to fix any errors — the check re-runs automatically.

Auto-labeling

Labels are applied automatically — you don't need to add them manually:

Type labels from your PR title (e.g., feat: adds feature, fix: adds bugfix)
Package labels from which files you changed (e.g., editing packages/syft-bg/ adds pkg:syft-bg)

These labels are used to auto-generate categorized release notes.

Copilot

Pull request overview

This PR introduces a new syft-rds “product layer” that composes the domain-free syft-client sync engine with the jobs/datasets managers, while refactoring the sync stack to support generic “collection” syncing (prefix + local subpath + policy) instead of being dataset-specific.

Changes:

Add syft-rds package (client/config/login + job auto-approval) and migrate consumers (syft-bg, syft-enclave, tests) to use it.
Refactor syft-client sync to support prefix-parameterized collection APIs (transport/router/cache/syncers) and a new CollectionSyncSpec.
Adjust dependencies/entrypoints to reflect the split between generic sync core and RDS product functionality.

Reviewed changes

Copilot reviewed 49 out of 49 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tests/unit/syft_bg/test_email_auto_approve_flow.py	Switch syft-bg email flow test to use `SyftRDSClient` (currently calls a non-existent helper).
tests/unit/syft_bg/test_email_approval_flow.py	Switch syft-bg approval flow test to use `SyftRDSClient` (currently calls a non-existent helper).
syft_client/sync/sync/datasite_watcher_syncer.py	Rename/retarget watcher sync-down from dataset files to generic collection files.
syft_client/sync/sync/datasite_owner_syncer.py	Generalize owner initial restore/download logic to iterate `CollectionSyncSpec` entries.
syft_client/sync/sync/collection_spec.py	New domain-agnostic collection sync contract (prefix/subpath + immutable/owner-only flags).
syft_client/sync/sync/caches/datasite_watcher_cache.py	Generalize watcher cache from dataset collections to multi-spec collection syncing + hash cache.
syft_client/sync/sync/caches/datasite_owner_cache.py	Relax collections folder requirement and allow returning `None` for collections-relative path.
syft_client/sync/syftbox_manager.py	Strip job/dataset responsibilities from sync engine; add callback emission for peer lifecycle.
syft_client/sync/connections/drive/gdrive_transport.py	Generalize Google Drive collection folder naming/parsing + CRUD/download APIs by prefix.
syft_client/sync/connections/connection_router.py	Replace dataset-specific router methods with prefix-parameterized collection methods.
syft_client/sync/connections/collection_prefixes.py	Add canonical wire-level collection folder prefixes.
syft_client/sync/connections/base_connection.py	Update base connection interface to prefix-parameterized collection methods.
syft_client/sync/callback_mixin.py	Add `_emit` and `on()` alias to support safe lifecycle callbacks.
scripts/auto_approve_jobs.py	Update script entrypoint to use `syft_rds.login_do` and RDS job auto-approval.
pyproject.toml	Remove several deps from the root project (jobs/datasets now owned by syft-rds).
packages/syft-rds/tests/test_load_dataset_code.py	Update RDS product tests to use `SyftRDSClient` for pairing.
packages/syft-rds/tests/test_job_auto_approval.py	Move auto-approval tests to RDS product client + module.
packages/syft-rds/tests/test_datasets_jobs_repr.py	Update repr tests to use RDS pairing and shared test utils.
packages/syft-rds/tests/test_dataset_upload_private.py	Update private dataset upload tests to use RDS client and generic collection APIs.
packages/syft-rds/tests/test_create_dataset_cleanup.py	Update create_dataset cleanup tests to use RDS client and updated logging.
packages/syft-rds/tests/test_client.py	Add coverage ensuring RDS injects collection specs into the composed sync engine.
packages/syft-rds/tests/dataset_test_utils.py	Add uniquely-named shared helpers to avoid pytest module-name collisions.
packages/syft-rds/tests/conftest.py	Add per-suite pytest config mirroring top-level behavior (disable PRE_SYNC).
packages/syft-rds/src/syft_rds/login.py	Add RDS login entrypoints (`login_do`, `login_ds`) wrapping the sync-engine login flow.
packages/syft-rds/src/syft_rds/job_auto_approval.py	Re-home job auto-approval to RDS client surface/types.
packages/syft-rds/src/syft_rds/config.py	Add composed RDS config (sync + job + dataset) and dataset collection specs injection.
packages/syft-rds/src/syft_rds/client.py	Introduce `SyftRDSClient` composing sync engine with job/dataset managers + callbacks.
packages/syft-rds/src/syft_rds/init.py	Define public RDS API surface exports.
packages/syft-rds/pyproject.toml	Add `syft-rds` package metadata and workspace dependencies.
packages/syft-enclave/tests/test_runner.py	Update enclave runner tests to call `client.delete_syftbox()` directly.
packages/syft-enclave/tests/test_immutability.py	Update enclave immutability tests to use nested RDS client surfaces.
packages/syft-enclave/tests/test_encryption.py	Update encryption tests to access peer store via `rds.sync_engine`.
packages/syft-enclave/tests/test_enclave_datasets.py	Update enclave dataset tests to use RDS client fields.
packages/syft-enclave/tests/test_enclave_client.py	Update enclave client tests for new RDS-composed structure.
packages/syft-enclave/src/syft_enclaves/utils.py	Switch enclave test config creation to `SyftRDSClientConfig`.
packages/syft-enclave/src/syft_enclaves/runner.py	Route lifecycle actions through enclave client public methods / RDS peer manager.
packages/syft-enclave/src/syft_enclaves/login.py	Use `SyftRDSClientConfig` and run sync-engine login init via nested sync engine.
packages/syft-enclave/src/syft_enclaves/client.py	Refactor enclave client to wrap `SyftRDSClient` and delegate appropriately.
packages/syft-enclave/pyproject.toml	Add dependency on `syft-rds`.
packages/syft-bg/src/syft_bg/sync/orchestrator.py	Build orchestrator clients via `SyftRDSClientConfig`/`SyftRDSClient`.
packages/syft-bg/src/syft_bg/notify/handlers/job.py	Import `is_normal_syncable_path` from syft-rds re-export.
packages/syft-bg/src/syft_bg/common/syft_bg_config.py	Use syft-rds re-export for default syftbox root resolution.
packages/syft-bg/src/syft_bg/cli/init/flow.py	Use syft-rds re-export for default syftbox root resolution.
packages/syft-bg/src/syft_bg/approve/orchestrator.py	Build approval orchestrator client via `SyftRDSClientConfig`/`SyftRDSClient`.
packages/syft-bg/src/syft_bg/approve/monitors/peer.py	Update type hints from `SyftboxManager` to `SyftRDSClient`.
packages/syft-bg/src/syft_bg/approve/monitors/job.py	Update type hints from `SyftboxManager` to `SyftRDSClient`.
packages/syft-bg/src/syft_bg/approve/handlers/peer.py	Update type hints from `SyftboxManager` to `SyftRDSClient`.
packages/syft-bg/src/syft_bg/approve/handlers/job.py	Update type hints from `SyftboxManager` to `SyftRDSClient`.
packages/syft-bg/pyproject.toml	Add dependency on `syft-rds`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

rasswanth-s · 2026-07-15T12:29:47Z

+# Generic sync-stack helpers re-exported so consumers that compose the RDS product
+# (e.g. syft-bg) have a single integration surface and need not import from
+# syft_client.sync internals directly.
+from syft_client.sync.syftbox_manager import get_jupyter_default_syftbox_folder


This pattern of importing syft-client imports at rds is temporary, will refactor this part shortly

pjwerneck

The intent to create a domain-free core and define a public API is clear, but there are still violations of that boundary. It's mostly clear for the happy path, but there's still coupling and private-reaching when handling others cases.

There are still broken tests and conflicts, so this review is not final.

pjwerneck · 2026-07-17T12:35:19Z

+    PRIVATE_DATASET_COLLECTION_PREFIX,
+)
+
+# The RDS layer OWNS the dataset collection values: both the on-wire prefixes and


The subpaths are defined below, but the prefixes are being imported from syft_datasets.dataset_manager. This comment seems contradictory.

pjwerneck · 2026-07-17T12:44:54Z

+
+        ``sync`` is built by the caller (it differs per environment); ``job``
+        and ``dataset`` are derived here from the SAME email/folder/role so all
+        paths line up with the sync engine.


The alignment is documented, but there's no validator enforcing it. Like a @model_validator(mode="after") asserting that sync.email and dataset.email match.

pjwerneck · 2026-07-17T12:58:55Z

+            current_user_email=email,
+            has_do_role=has_do_role,
+        )
+        dataset = SyftBoxConfig(syftbox_folder=folder, email=email)


Why are we taking folder and email as parameters here, instead of using attributes from sync? It looks like a potential source of drift or inconsistencies.

pjwerneck · 2026-07-17T13:20:08Z

+        so explicit user sharing is added. Reads the cache populated during
+        ``pull_initial_state()`` in the nested DatasiteOwnerSyncer.
+        """
+        for tag, content_hash in self._sync.datasite_owner_syncer._any_shared_collections:


This is one example of the encapsulation violations.

pjwerneck · 2026-07-17T13:35:03Z

+                self._sync._connection_router.owner_share_collection(
+                    DATASET_COLLECTION_PREFIX, tag, content_hash, [peer_email]
+                )
+            except Exception:


This doesn't look like an acceptable "ignore all errors" case as the comment suggests, since owner_share_collection is not a trivial method. If a network request fails, it's going to behave as if it's already shared. This could also potentially silently real bugs in owner_share_collection.

Also, using a bare except: or a blanket except Exception: to silently suppress all errors is a python anti-pattern. In legitimate cases, contextlib.suppress is the more idiomatic way of doing that.

pjwerneck · 2026-07-17T14:20:57Z

+                private_folder_id = self._upload_private_dataset_to_collection(dataset)
+
+            if sync:
+                self.sync()


The word "sync" is carrying four different meanings in the file. That's a real readability issue. In most cases it refers to the SyftboxManager instance, but it's also used as a boolean flag meaning to run a full sync after the method, the sync() method itself, that delegates to self._sync.sync() , and config.sync referring to the sub-config.

I'd rename the boolean to something more explicit, like sync_after, and the private attribute to _sync_engine so it matches the public property sync_engine.

pjwerneck · 2026-07-17T14:37:57Z

        syftbox_folder = get_colab_default_syftbox_folder(email)
        use_in_memory_cache = False
-        collections_folder = syftbox_folder / email / COLLECTION_SUBPATH
+        collections_folder = (


This interpolation is repeated four times in the file. It should be extracted to a common function/method.

Also, it's not clear why it assumes collection_specs[0] is special. Is it because it's the public spec? What happens if they are reordered? It should have a clarifying comment here.

If it must always be the public, it should refer to it by an unambiguous value.

pjwerneck · 2026-07-17T14:44:01Z

        """
        cast(PeerManager, self.peer_manager).load_peers(force_download=force_download)
-        self._sync_peer_install_sources_to_job_client()
+        self._emit_peers_loaded()


The sync method calls load_peers on every invocation, so every sync emits peers_loaded. Is that intentional? Is there any idempotency handling?

pjwerneck · 2026-07-17T14:46:16Z

        self.peer_manager.add_peer(peer_email, force=force, verbose=verbose)
-        self._sync_peer_install_sources_to_job_client()
+        self._emit_peers_loaded()
        if self.has_do_role:


Does this mean a peer added by the DO is approved automatically? If that's the case, there should be a comment clarifying it.

pjwerneck · 2026-07-17T14:50:14Z

+        self._emit_peers_loaded()
        if self.has_do_role:
            self._post_approve_peer_do(peer_email)
        if sync:


This is the same issue with the word "sync" mentioned in client.py. For clarity, the boolean could be renamed to sync_after.

rasswanth-s added 15 commits July 2, 2026 10:21

initial stub rds package

210f2bc

update import names

fc3363f

add callbacks to syftbox manager

8293a8f

modify collections to be generic

3d96401

modify base colletion objects

94d88d0

remove job/dataset refeerences in syftbox_manager

6e89e60

shift bg to use syft-rds client

9d8f877

[encalves]shift enclaves package to use syft-rds client

f4c92ae

[encalves]fix enclaves testing

fe9341d

remove syft-client as dep from syft-bg

63491fe

[syft-bg] fix unit tests class naming

0127295

[syft-client] make collection generic

e2b52bd

[syft-client] shift auto approval to syft-rds

55587f7

Add initial RDSClient and Config implementations

78f10be

shift to generic collection folder name

0a1f50c

github-actions Bot added feature pkg:syft-enclave pkg:syft-client pkg:syft-bg labels Jul 8, 2026

rasswanth-s added 9 commits July 9, 2026 10:26

[syft-client] lean syft-client dependencies

91556c9

add RDS unit tests

48f51b9

re-order batch collections

8167b7b

parametrize collection folder names

d4977f4

re-order collection sync spec

f963417

re-order imports

717b7b8

add exception handling to callbacks

f82ed84

shift location of peers loaded callback

09dc366

refactor lower level abstract methods in syft enclave client

3bc4fa0

rasswanth-s requested review from Copilot and koenvanderveen July 15, 2026 10:54

Copilot started reviewing on behalf of rasswanth-s July 15, 2026 10:54 View session

rasswanth-s changed the title ~~feat: [WIP] Init RDS package~~ feat: Init RDS package Jul 15, 2026

Copilot AI reviewed Jul 15, 2026

View reviewed changes

Copilot started work on behalf of rasswanth-s July 15, 2026 11:05 View session

fix: address PR review thread issues

40bf126

Copilot finished work on behalf of rasswanth-s July 15, 2026 11:10

rasswanth-s commented Jul 15, 2026

View reviewed changes

pjwerneck reviewed Jul 17, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

rasswanth-s commented Jul 8, 2026

Summary

Changes

Testing

Asana task

PR naming convention

Auto-labeling

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

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!

4 participants