diff --git a/tests/trace/test_weave_client.py b/tests/trace/test_weave_client.py index 32de644d839..c13314878d4 100644 --- a/tests/trace/test_weave_client.py +++ b/tests/trace/test_weave_client.py @@ -1544,3 +1544,147 @@ def func(): calls = list(func.calls()) call = calls[0] assert len(call.display_name) <= MAX_DISPLAY_NAME_LENGTH + + +def test_object_deletion(client): + # Simple case, delete a single version of an object + obj = {"a": 5} + weave_obj = client.save(obj, "my-obj") + assert client.get(weave_obj.ref) == obj + + client.delete_object_version(weave_obj.ref) + with pytest.raises(weave.trace_server.errors.ObjectDeletedError): + client.get(weave_obj.ref) + + # create 3 versions of the object + obj["a"] = 6 + weave_obj2 = client.save(obj, "my-obj") + obj["a"] = 7 + weave_obj3 = client.save(obj, "my-obj") + obj["a"] = 8 + weave_obj4 = client.save(obj, "my-obj") + + # delete weave_obj3 with class method + weave_obj3.delete() + + # make sure we can't get the deleted object + with pytest.raises(weave.trace_server.errors.ObjectDeletedError): + client.get(weave_obj3.ref) + + # make sure we can still get the existing object versions + assert client.get(weave_obj4.ref) + assert client.get(weave_obj2.ref) + + # count the number of versions of the object + versions = client.server.objs_query( + req=tsi.ObjQueryReq( + project_id=client._project_id(), + names=["my-obj"], + sort_by=[tsi.SortBy(field="created_at", direction="desc")], + ) + ) + assert len(versions.objs) == 2 + + # iterate over the versions, confirm the indexes are correct + assert versions.objs[0].version_index == 3 + assert versions.objs[1].version_index == 1 + + weave_obj4.delete() + weave_obj2.delete() + + versions = client.server.objs_query( + req=tsi.ObjQueryReq( + project_id=client._project_id(), + names=["my-obj"], + ) + ) + assert len(versions.objs) == 0 + + +def test_recursive_object_deletion(client): + # Create a bunch of objects that refer to each other + obj1 = {"a": 5} + obj1_ref = client.save(obj1, "obj1").ref + + obj2 = {"b": obj1_ref} + obj2_ref = client.save(obj2, "obj2").ref + + obj3 = {"c": obj2_ref} + obj3_ref = client.save(obj3, "obj3").ref + + # Delete obj1 + client.get(obj1_ref).delete() + + # Make sure we can't get obj1 + with pytest.raises(weave.trace_server.errors.ObjectDeletedError): + out = client.get(obj1_ref) + print(out) + + # Make sure we can get obj2, but the ref to object 1 should return None + assert client.get(obj2_ref) == {"b": None} + # Object2 should store the ref to object2, as instantiated + assert client.get(obj3_ref) == {"c": obj2} + + +def test_delete_all_versions(client): + obj = {"a": 5} + weave_obj = client.save(obj, "my-obj") + assert client.get(weave_obj.ref) == obj + + client.delete_object_all_versions(weave_obj.ref) + with pytest.raises(weave.trace_server.errors.ObjectDeletedError): + client.get(weave_obj.ref) + + # create 10 versions of the object + for i in range(10): + obj["a"] = i + weave_obj = client.save(obj, "my-obj-2") + + client.delete_object_all_versions(weave_obj.ref) + objs = client.server.objs_query( + req=tsi.ObjQueryReq( + project_id=client._project_id(), + names=["my-obj-2"], + ) + ).objs + assert len(objs) == 0 + + +@pytest.mark.skip("Deleting tables is not yet supported.") +def test_table_object_deletion(client): + # create a dataset + data = [{"a": 1}, {"a": 2}, {"a": 3}] + dataset = weave.Table(data) + dataset_ref = client.save(dataset, "my-dataset").ref + + # get the dataset + assert client.get(dataset_ref) == dataset + + # delete the dataset + client.delete_object_version(dataset_ref) + + # make sure we can't get the dataset + with pytest.raises(weave.trace_server.errors.ObjectDeletedError): + client.get(dataset_ref) + + # recreate the EXACT SAME dataset + dataset = weave.Table(data) + dataset_ref = client.save(dataset, "my-dataset").ref + + assert client.get(dataset_ref) == dataset + read_res = client.server.obj_read( + tsi.ObjReadReq( + project_id=client._project_id(), + object_id=dataset_ref.name, + digest=dataset_ref.digest, + ) + ) + # Version should be bumped + assert read_res.obj.version_index == 1 + + # delete the dataset again + client.delete_object_version(dataset_ref) + + # make sure we can't get the dataset + with pytest.raises(weave.trace_server.errors.ObjectDeletedError): + client.get(dataset_ref) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse2/SmallRef.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse2/SmallRef.tsx index 40555a00f69..c7228309717 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse2/SmallRef.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse2/SmallRef.tsx @@ -125,6 +125,11 @@ export const SmallRef: FC<{ } const objectVersion = useObjectVersion(objVersionKey); const opVersion = useOpVersion(opVersionKey); + + const isDeleted = + isObjDeleteError(objectVersion?.error) || + isObjDeleteError(opVersion?.error); + const versionIndex = objectVersion.result?.versionIndex ?? opVersion.result?.versionIndex; @@ -140,7 +145,6 @@ export const SmallRef: FC<{ rootType = {type: 'OpDef'}; } const {label} = objectRefDisplayName(objRef, versionIndex); - const rootTypeName = getTypeName(rootType); let icon: IconName = IconNames.CubeContainer; if (rootTypeName === 'Dataset') { @@ -177,13 +181,14 @@ export const SmallRef: FC<{ overflow: 'hidden', whiteSpace: 'nowrap', textOverflow: 'ellipsis', + textDecoration: isDeleted ? 'line-through' : 'none', }}> {label} )} ); - if (refTypeQuery.loading) { + if (refTypeQuery.loading || isDeleted) { return Item; } if (!isArtifactRef && !isWeaveObjRef) { @@ -200,3 +205,12 @@ export const SmallRef: FC<{ ); }; + +export const isObjDeleteError = (e: any): boolean => { + if (e == null) { + return false; + } + const errorStr = String(e); + const regex = /Obj .* was deleted at .*/; + return regex.test(errorStr); +}; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewer.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewer.tsx index e8f39ad2880..25148366517 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewer.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewer.tsx @@ -7,6 +7,7 @@ import { GridRowId, } from '@mui/x-data-grid-pro'; import {Button} from '@wandb/weave/components/Button'; +import {parseRef} from '@wandb/weave/react'; import _ from 'lodash'; import React, { Dispatch, @@ -21,6 +22,7 @@ import React, { import {parseRefMaybe} from '../../../../../../react'; import {LoadingDots} from '../../../../../LoadingDots'; import {Browse2OpDefCode} from '../../../Browse2/Browse2OpDefCode'; +import {objectRefDisplayName} from '../../../Browse2/SmallRef'; import {isWeaveRef} from '../../filters/common'; import {StyledDataGrid} from '../../StyledDataGrid'; import { @@ -161,10 +163,17 @@ export const ObjectViewer = ({ const refValues: RefValues = {}; for (const [r, v] of _.zip(refs, resolvedRefData)) { - if (!r || !v) { + if (!r) { // Shouldn't be possible continue; } + if (!v) { + // Value for ref not found, probably deleted + refValues[r] = { + _weave_is_deleted_ref: objectRefDisplayName(parseRef(r)).label, + }; + continue; + } let val = r; if (v == null) { console.error('Error resolving ref', r); diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewerGroupingCell.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewerGroupingCell.tsx index 2aeb6309afa..37e1a3d5f7b 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewerGroupingCell.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewerGroupingCell.tsx @@ -32,7 +32,9 @@ export const ObjectViewerGroupingCell: FC< event.stopPropagation(); }; + const deletedRef = row.value?._weave_is_deleted_ref; const tooltipContent = row.path.toString(); + const textContent = deletedRef ?? props.value; const box = ( - {props.value} + {textContent} } /> diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx index 085587a64d8..14aafb9893c 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx @@ -12,6 +12,7 @@ import {CustomWeaveTypeProjectContext} from '../typeViews/CustomWeaveTypeDispatc import {WeaveCHTableSourceRefContext} from './CallPage/DataTableView'; import {ObjectViewerSection} from './CallPage/ObjectViewerSection'; import {WFHighLevelCallFilter} from './CallsPage/callsTableFilter'; +import {DeleteObjectButtonWithModal} from './common/DeleteModal'; import { CallLink, CallsLink, @@ -94,7 +95,9 @@ export const ObjectVersionPage: React.FC<{ path: props.filePath, refExtra: props.refExtra, }); - if (objectVersion.loading) { + if (objectVersion.error) { + return ; + } else if (objectVersion.loading) { return ; } else if (objectVersion.result == null) { return ; @@ -212,7 +215,7 @@ const ObjectVersionPageInner: React.FC<{ } headerContent={ -
+

Name

@@ -253,6 +256,9 @@ const ObjectVersionPageInner: React.FC<{

{refExtra}

)} +
+ +
} diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx index 36f4e44afc5..13c878f0c8c 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx @@ -5,6 +5,7 @@ import {LoadingDots} from '../../../../LoadingDots'; import {Tailwind} from '../../../../Tailwind'; import {NotFoundPanel} from '../NotFoundPanel'; import {OpCodeViewer} from '../OpCodeViewer'; +import {DeleteObjectButtonWithModal} from './common/DeleteModal'; import { CallsLink, opNiceName, @@ -76,7 +77,7 @@ const OpVersionPageInner: React.FC<{ title={opVersionText(opId, versionIndex)} headerContent={ -
+

Name

@@ -138,6 +139,9 @@ const OpVersionPageInner: React.FC<{

-

)}
+
+ +
} diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx new file mode 100644 index 00000000000..063008145ed --- /dev/null +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx @@ -0,0 +1,174 @@ +import { + Dialog as MaterialDialog, + DialogActions as MaterialDialogActions, + DialogContent as MaterialDialogContent, + DialogTitle as MaterialDialogTitle, +} from '@material-ui/core'; +import {Button} from '@wandb/weave/components/Button'; +import {Tailwind} from '@wandb/weave/components/Tailwind'; +import React, {useState} from 'react'; +import styled from 'styled-components'; + +import {useClosePeek} from '../../context'; +import {useWFHooks} from '../wfReactInterface/context'; +import { + ObjectVersionSchema, + OpVersionSchema, +} from '../wfReactInterface/wfDataModelHooksInterface'; + +interface DeleteModalProps { + open: boolean; + onClose: () => void; + onDelete: () => Promise; + deleteTargetStr: string; + onSuccess?: () => void; +} + +const Dialog = styled(MaterialDialog)` + .MuiDialog-paper { + min-width: 400px; + max-width: min(800px, 90vw); + width: auto !important; + } +`; + +export const DeleteModal: React.FC = ({ + open, + onClose, + onDelete, + deleteTargetStr, + onSuccess, +}) => { + const [deleteLoading, setDeleteLoading] = useState(false); + const [error, setError] = useState(null); + + const handleDelete = () => { + setDeleteLoading(true); + onDelete() + .then(() => { + onClose(); + onSuccess?.(); + }) + .catch(err => { + setError(err.message); + }) + .finally(() => { + setDeleteLoading(false); + }); + }; + + return ( + { + onClose(); + setError(null); + }}> + + Delete {deleteTargetStr} + +
+ {error != null ? ( +

{error}

+ ) : ( +

Are you sure you want to delete?

+ )} +
+ {deleteTargetStr} + + + + + + + + ); +}; + +const DialogContent = styled(MaterialDialogContent)` + padding: 0 32px !important; +`; +DialogContent.displayName = 'S.DialogContent'; + +const DialogTitle = styled(MaterialDialogTitle)` + padding: 32px 32px 16px 32px !important; + + h2 { + font-weight: 600; + font-size: 24px; + line-height: 30px; + } +`; +DialogTitle.displayName = 'S.DialogTitle'; + +const DialogActions = styled(MaterialDialogActions)<{$align: string}>` + justify-content: ${({$align}) => + $align === 'left' ? 'flex-start' : 'flex-end'} !important; + padding: 32px 32px 32px 32px !important; +`; +DialogActions.displayName = 'S.DialogActions'; + +export const DeleteObjectButtonWithModal: React.FC<{ + objVersionSchema: OpVersionSchema | ObjectVersionSchema; + overrideDisplayStr?: string; +}> = ({objVersionSchema, overrideDisplayStr}) => { + const {useObjectDeleteFunc} = useWFHooks(); + const closePeek = useClosePeek(); + const {opVersionDelete, objectVersionDelete} = useObjectDeleteFunc(); + const [deleteModalOpen, setDeleteModalOpen] = useState(false); + + const doDelete = () => { + if (versionSchemaIsOp(objVersionSchema)) { + return opVersionDelete(objVersionSchema); + } + return objectVersionDelete(objVersionSchema); + }; + + const deleteStr = + overrideDisplayStr ?? makeDefaultObjectDeleteStr(objVersionSchema); + + return ( + <> +