Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Level 0 UX for low side decision making #9315

Merged
merged 11 commits into from
Sep 26, 2024
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,5 @@ notebooks/scenarios/bigquery/sync/*.json
notebooks/scenarios/bigquery/sync/*.json.lock
notebooks/tutorials/version-upgrades/*.yaml
notebooks/tutorials/version-upgrades/*.blob

notebooks/scenarios/bigquery/sync/emails.json
4 changes: 2 additions & 2 deletions notebooks/scenarios/bigquery/010-setup-bigquery-pool.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@
],
"metadata": {
"kernelspec": {
"display_name": "syft",
"display_name": "Python 3 (ipykernel)",
"language": "python",
"name": "python3"
},
Expand All @@ -536,7 +536,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.12.3"
"version": "3.12.5"
}
},
"nbformat": 4,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.12.4"
"version": "3.12.5"
}
},
"nbformat": 4,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@
],
"metadata": {
"kernelspec": {
"display_name": "syft",
"display_name": "Python 3 (ipykernel)",
"language": "python",
"name": "python3"
},
Expand All @@ -329,7 +329,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.12.4"
"version": "3.12.5"
}
},
"nbformat": 4,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.12.4"
"version": "3.12.5"
}
},
"nbformat": 4,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.12.4"
"version": "3.12.5"
}
},
"nbformat": 4,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.12.4"
"version": "3.12.5"
}
},
"nbformat": 4,
Expand Down
26 changes: 24 additions & 2 deletions notebooks/scenarios/bigquery/sync/021-create-jobs.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@
"id": "32",
"metadata": {},
"source": [
"# Cleanup"
"# Submit a broken query"
]
},
{
Expand All @@ -387,6 +387,28 @@
"id": "33",
"metadata": {},
"outputs": [],
"source": [
"ds_client = users[0].client\n",
"submission = ds_client.api.services.bigquery.submit_query(\n",
" func_name=\"broken_query\", query=\"BROKEN QUERY\"\n",
")\n",
"submission"
]
},
{
"cell_type": "markdown",
"id": "34",
"metadata": {},
"source": [
"# Cleanup"
]
},
{
"cell_type": "code",
"execution_count": null,
"id": "35",
"metadata": {},
"outputs": [],
"source": [
"if environment != \"remote\":\n",
" server_low.land()\n",
Expand All @@ -410,7 +432,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.12.4"
"version": "3.12.5"
}
},
"nbformat": 4,
Expand Down
103 changes: 78 additions & 25 deletions notebooks/scenarios/bigquery/sync/040-do-review-requests.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"outputs": [],
"source": [
"# stdlib\n",
"from collections import Counter\n",
"import os\n",
"\n",
"# third party\n",
Expand All @@ -19,7 +20,8 @@
"from syft.util.test_helpers.email_helpers import load_users\n",
"from syft.util.test_helpers.job_helpers import get_job_emails\n",
"from syft.util.test_helpers.job_helpers import get_request_for_job_info\n",
"from syft.util.test_helpers.job_helpers import load_jobs"
"from syft.util.test_helpers.job_helpers import load_jobs\n",
"from syft.util.test_helpers.job_helpers import save_jobs"
]
},
{
Expand Down Expand Up @@ -106,9 +108,14 @@
"metadata": {},
"outputs": [],
"source": [
"diffs = compare_clients(\n",
" from_client=low_client, to_client=high_client, hide_usercode=False\n",
")"
"widget = sy.sync(low_client, high_client)"
]
},
{
"cell_type": "markdown",
"metadata": {},
"source": [
"# Ignore batches we dont want to sync"
]
},
{
Expand All @@ -117,10 +124,28 @@
"metadata": {},
"outputs": [],
"source": [
"# check that only requests and usercode are in the diff\n",
"idxs_to_ignore = []\n",
"\n",
"for idx in range(len(widget)):\n",
" batch = widget[idx].obj_diff_batch\n",
" request = batch.root.low_obj\n",
" if request is not None and \"broken\" in request.code.service_func_name:\n",
" idxs_to_ignore.append(idx)\n",
"\n",
"for idx in idxs_to_ignore:\n",
" widget[idx].deny_and_ignore(\"query is broken\")"
]
},
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"diffs = compare_clients(low_client, high_client)\n",
"# # check that only requests and usercode are in the diff\n",
"assert {diff.root_diff.obj_type.__qualname__ for diff in diffs.batches} == {\n",
" \"Request\",\n",
" \"UserCode\",\n",
"}"
]
},
Expand All @@ -130,9 +155,8 @@
"metadata": {},
"outputs": [],
"source": [
"widget = diffs.resolve()\n",
"\n",
"widget._share_all()"
"# widget._share_all()\n",
"widget._sync_all()"
]
},
{
Expand All @@ -141,7 +165,17 @@
"metadata": {},
"outputs": [],
"source": [
"widget._sync_all()"
"# syft absolute\n",
"from syft.service.request.request import RequestStatus"
]
},
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"assert any(x.status == RequestStatus.REJECTED for x in low_client.requests)"
]
},
{
Expand Down Expand Up @@ -243,7 +277,7 @@
" )\n",
" assert isinstance(response, sy.SyftSuccess)\n",
" assert not job.should_succeed\n",
" job.admin_reviwed = True"
" job.admin_reviewed = True"
]
},
{
Expand All @@ -259,9 +293,7 @@
"metadata": {},
"outputs": [],
"source": [
"diffs = compare_clients(\n",
" from_client=high_client, to_client=low_client, hide_usercode=False\n",
")"
"widget = sy.sync(from_client=high_client, to_client=low_client)"
]
},
{
Expand All @@ -270,7 +302,8 @@
"metadata": {},
"outputs": [],
"source": [
"diffs.batches"
"diffs = sy.compare_clients(high_client, low_client)\n",
"batch_root_strs = [x.root_diff.obj_type.__qualname__ for x in diffs.batches]"
]
},
{
Expand All @@ -279,14 +312,21 @@
"metadata": {},
"outputs": [],
"source": [
"diffs = sy.compare_clients(high_client, low_client)\n",
"batch_root_strs = [x.root_diff.obj_type.__qualname__ for x in diffs.batches]\n",
"# for successful jobs, should be job, request, and usercode.\n",
"# For failed jobs, should be usercode, request only since job not run\n",
"expected_n_batches = 2 * len(submitted_jobs_data_should_fail) + 3 * len(\n",
" submitted_jobs_data_should_succeed\n",
")\n",
"assert len(diffs.batches) == expected_n_batches\n",
"assert \"Job\" in batch_root_strs"
"root_str_counts = Counter(batch_root_strs)\n",
"# for successful jobs, root diff should be job. Otherwise request\n",
"assert root_str_counts[\"Job\"] == len(submitted_jobs_data_should_succeed)\n",
"assert root_str_counts[\"Request\"] == len(submitted_jobs_data_should_fail)"
]
},
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"widget._share_all()"
]
},
{
Expand All @@ -295,12 +335,25 @@
"metadata": {},
"outputs": [],
"source": [
"widget = diffs.resolve()\n",
"\n",
"widget._share_all()\n",
"widget._sync_all()"
]
},
{
"cell_type": "markdown",
"metadata": {},
"source": [
"# Save state"
]
},
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"save_jobs(jobs_data)"
]
},
{
"cell_type": "markdown",
"metadata": {},
Expand Down
10 changes: 4 additions & 6 deletions notebooks/scenarios/bigquery/sync/050-ds-get-results.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,10 @@
" print(f\"> Checking job: {job.job_type} {job.func_name} for user {job.user_email}\")\n",
" api_method = job.code_method\n",
"\n",
" j = api_method(blocking=False)\n",
" res = j.wait()\n",
" if isinstance(res, sy.SyftError):\n",
" job.result_as_expected = True\n",
" else:\n",
" raise sy.SyftException(public_message=f\"failed, job didnt raise {type(j)}\")"
" with sy.raises(sy.SyftException):\n",
" j = api_method(blocking=False)\n",
" res = j.wait()\n",
" job.result_as_expected = True"
]
},
{
Expand Down
2 changes: 2 additions & 0 deletions packages/syft/src/syft/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@
from .util.autoreload import enable_autoreload
from .util.commit import __commit__
from .util.patch_ipython import patch_ipython
from .util.reset_server import make_copy
from .util.reset_server import restore_copy
from .util.telemetry import instrument
from .util.telemetry import instrument_threads
from .util.util import autocache
Expand Down
13 changes: 8 additions & 5 deletions packages/syft/src/syft/service/code/user_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,14 +289,17 @@ def syft_get_diffs(self, ext_obj: Any) -> list[AttrDiff]:
from ...service.sync.diff_state import AttrDiff

diff_attrs = []
status = list(self.status_dict.values())[0]
ext_status = list(ext_obj.status_dict.values())[0]
approval_decision = list(self.status_dict.values())[0]
ext_approval_decision = list(ext_obj.status_dict.values())[0]

if status != ext_status:
if (
approval_decision.status != ext_approval_decision.status
or approval_decision.reason != ext_approval_decision.reason
):
diff_attr = AttrDiff(
attr_name="status_dict",
low_attr=status,
high_attr=ext_status,
low_attr=approval_decision,
high_attr=ext_approval_decision,
)
diff_attrs.append(diff_attr)
return diff_attrs
Expand Down
Loading
Loading