From fbb11a8d92d05d09e7f2bea551a61db8cb26914e Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Wed, 4 Sep 2024 17:40:18 -0700 Subject: [PATCH 01/59] wip --- weave/trace/weave_client.py | 6 ++++ weave/trace_server/clickhouse_schema.py | 18 +++++++++++ .../clickhouse_trace_server_batched.py | 22 +++++++++++-- weave/trace_server/trace_server_interface.py | 31 +++++++++++++++++++ 4 files changed, 75 insertions(+), 2 deletions(-) diff --git a/weave/trace/weave_client.py b/weave/trace/weave_client.py index 3c545a43d57..5b8d3a861a9 100644 --- a/weave/trace/weave_client.py +++ b/weave/trace/weave_client.py @@ -45,6 +45,7 @@ ObjReadReq, ObjSchema, ObjSchemaForInsert, + ObjsDeleteReq, Query, RefsReadBatchReq, StartedCallSchemaForInsert, @@ -752,6 +753,11 @@ def feedback( query=query, reaction=reaction, offset=offset, limit=limit ) + def delete_objects(self, object_ids: list[str]) -> None: + self.server.objs_delete( + ObjsDeleteReq(project_id=self._project_id(), object_ids=object_ids) + ) + ################ Internal Helpers ################ def _ref_is_own(self, ref: Ref) -> bool: diff --git a/weave/trace_server/clickhouse_schema.py b/weave/trace_server/clickhouse_schema.py index 1c0e3e62db8..99e51337d9d 100644 --- a/weave/trace_server/clickhouse_schema.py +++ b/weave/trace_server/clickhouse_schema.py @@ -151,3 +151,21 @@ class SelectableCHObjSchema(BaseModel): digest: str version_index: int is_latest: int + + +class ObjsDeleteCHInsertable(BaseModel): + project_id: str + object_id: typing.List[str] + wb_user_id: str + + deleted_at: datetime.datetime + + # required types + input_refs: typing.List[str] = Field(default_factory=list) + output_refs: typing.List[str] = Field(default_factory=list) + + _project_id_v = field_validator("project_id")(validation.project_id_validator) + _object_id_v = field_validator("object_id")(validation.object_id_validator) + _wb_user_id_v = field_validator("wb_user_id")(validation.wb_user_id_validator) + _input_refs_v = field_validator("input_refs")(validation.refs_list_validator) + _output_refs_v = field_validator("output_refs")(validation.refs_list_validator) diff --git a/weave/trace_server/clickhouse_trace_server_batched.py b/weave/trace_server/clickhouse_trace_server_batched.py index fd8ec1f6541..2b1b4be1f1d 100644 --- a/weave/trace_server/clickhouse_trace_server_batched.py +++ b/weave/trace_server/clickhouse_trace_server_batched.py @@ -66,6 +66,7 @@ CallStartCHInsertable, CallUpdateCHInsertable, ObjCHInsertable, + ObjsDeleteCHInsertable, SelectableCHCallSchema, SelectableCHObjSchema, ) @@ -615,6 +616,22 @@ def objs_query(self, req: tsi.ObjQueryReq) -> tsi.ObjQueryRes: return tsi.ObjQueryRes(objs=[_ch_obj_to_obj_schema(obj) for obj in objs]) + def objs_delete(self, req: tsi.ObjsDeleteReq) -> tsi.ObjsDeleteRes: + deleted_at = datetime.datetime.now() + for object_id in req.object_ids: + ch_obj = ObjsDeleteCHInsertable( + project_id=req.project_id, + object_id=object_id, + deleted_at=deleted_at, + ) + + self._insert( + "object_versions", + data=[list(ch_obj.model_dump().values())], + column_names=list(ch_obj.model_fields.keys()), + ) + return tsi.ObjsDeleteRes() + def table_create(self, req: tsi.TableCreateReq) -> tsi.TableCreateRes: insert_rows = [] for r in req.table.rows: @@ -1320,11 +1337,12 @@ def _select_objs_query( PARTITION BY project_id, kind, object_id, - digest + digest, + deleted_at ORDER BY created_at ASC ) AS rn FROM object_versions - WHERE project_id = {{project_id: String}} + WHERE project_id = {{project_id: String}} AND deleted_at IS NULL ) WHERE rn = 1 ) diff --git a/weave/trace_server/trace_server_interface.py b/weave/trace_server/trace_server_interface.py index 5447ba8df5e..357fa648615 100644 --- a/weave/trace_server/trace_server_interface.py +++ b/weave/trace_server/trace_server_interface.py @@ -375,6 +375,15 @@ class ObjQueryRes(BaseModel): objs: List[ObjSchema] +class ObjsDeleteReq(BaseModel): + project_id: str + object_ids: List[str] + + +class ObjsDeleteRes(BaseModel): + pass + + class TableCreateReq(BaseModel): table: TableSchemaForInsert @@ -598,31 +607,53 @@ def ensure_project_exists( # Call API def call_start(self, req: CallStartReq) -> CallStartRes: ... + def call_end(self, req: CallEndReq) -> CallEndRes: ... + def call_read(self, req: CallReadReq) -> CallReadRes: ... + def calls_query(self, req: CallsQueryReq) -> CallsQueryRes: ... + def calls_query_stream(self, req: CallsQueryReq) -> Iterator[CallSchema]: ... + def calls_delete(self, req: CallsDeleteReq) -> CallsDeleteRes: ... + def calls_query_stats(self, req: CallsQueryStatsReq) -> CallsQueryStatsRes: ... + def call_update(self, req: CallUpdateReq) -> CallUpdateRes: ... # Op API def op_create(self, req: OpCreateReq) -> OpCreateRes: ... + def op_read(self, req: OpReadReq) -> OpReadRes: ... + def ops_query(self, req: OpQueryReq) -> OpQueryRes: ... # Obj API def obj_create(self, req: ObjCreateReq) -> ObjCreateRes: ... + def obj_read(self, req: ObjReadReq) -> ObjReadRes: ... + def objs_query(self, req: ObjQueryReq) -> ObjQueryRes: ... + + def objs_delete(self, req: ObjsDeleteReq) -> ObjsDeleteRes: ... + def table_create(self, req: TableCreateReq) -> TableCreateRes: ... + def table_update(self, req: TableUpdateReq) -> TableUpdateRes: ... + def table_query(self, req: TableQueryReq) -> TableQueryRes: ... + def refs_read_batch(self, req: RefsReadBatchReq) -> RefsReadBatchRes: ... + def file_create(self, req: FileCreateReq) -> FileCreateRes: ... + def file_content_read(self, req: FileContentReadReq) -> FileContentReadRes: ... + def feedback_create(self, req: FeedbackCreateReq) -> FeedbackCreateRes: ... + def feedback_query(self, req: FeedbackQueryReq) -> FeedbackQueryRes: ... + def feedback_purge(self, req: FeedbackPurgeReq) -> FeedbackPurgeRes: ... From 83ac64572f1cd4cc8fb077f11ac56b0a99a39b7b Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Thu, 5 Sep 2024 10:17:10 -0700 Subject: [PATCH 02/59] wip --- weave/trace_server/clickhouse_schema.py | 8 +------- weave/trace_server/clickhouse_trace_server_batched.py | 2 ++ weave/trace_server/trace_server_interface.py | 5 ++++- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/weave/trace_server/clickhouse_schema.py b/weave/trace_server/clickhouse_schema.py index 99e51337d9d..c203a518f30 100644 --- a/weave/trace_server/clickhouse_schema.py +++ b/weave/trace_server/clickhouse_schema.py @@ -155,17 +155,11 @@ class SelectableCHObjSchema(BaseModel): class ObjsDeleteCHInsertable(BaseModel): project_id: str - object_id: typing.List[str] + object_id: str wb_user_id: str deleted_at: datetime.datetime - # required types - input_refs: typing.List[str] = Field(default_factory=list) - output_refs: typing.List[str] = Field(default_factory=list) - _project_id_v = field_validator("project_id")(validation.project_id_validator) _object_id_v = field_validator("object_id")(validation.object_id_validator) _wb_user_id_v = field_validator("wb_user_id")(validation.wb_user_id_validator) - _input_refs_v = field_validator("input_refs")(validation.refs_list_validator) - _output_refs_v = field_validator("output_refs")(validation.refs_list_validator) diff --git a/weave/trace_server/clickhouse_trace_server_batched.py b/weave/trace_server/clickhouse_trace_server_batched.py index 2b1b4be1f1d..e7aa7bd86d6 100644 --- a/weave/trace_server/clickhouse_trace_server_batched.py +++ b/weave/trace_server/clickhouse_trace_server_batched.py @@ -617,10 +617,12 @@ def objs_query(self, req: tsi.ObjQueryReq) -> tsi.ObjQueryRes: return tsi.ObjQueryRes(objs=[_ch_obj_to_obj_schema(obj) for obj in objs]) def objs_delete(self, req: tsi.ObjsDeleteReq) -> tsi.ObjsDeleteRes: + assert_non_null_wb_user_id(req) deleted_at = datetime.datetime.now() for object_id in req.object_ids: ch_obj = ObjsDeleteCHInsertable( project_id=req.project_id, + wb_user_id=req.wb_user_id, object_id=object_id, deleted_at=deleted_at, ) diff --git a/weave/trace_server/trace_server_interface.py b/weave/trace_server/trace_server_interface.py index 357fa648615..802ba5a362f 100644 --- a/weave/trace_server/trace_server_interface.py +++ b/weave/trace_server/trace_server_interface.py @@ -376,8 +376,11 @@ class ObjQueryRes(BaseModel): class ObjsDeleteReq(BaseModel): + # wb_user_id is automatically populated by the server + wb_user_id: Optional[str] = Field(None, description=WB_USER_ID_DESCRIPTION) + project_id: str - object_ids: List[str] + object_ids: str class ObjsDeleteRes(BaseModel): From 6c14fff1b66c9a6b95de750f4a1e2606241910ff Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Thu, 5 Sep 2024 13:42:41 -0700 Subject: [PATCH 03/59] wip, working?? --- weave/trace/weave_client.py | 12 ++++-- weave/trace_server/clickhouse_schema.py | 9 ++-- .../clickhouse_trace_server_batched.py | 43 +++++++++---------- ...ternal_to_internal_trace_server_adapter.py | 4 ++ weave/trace_server/sqlite_trace_server.py | 12 ++++++ weave/trace_server/trace_server_interface.py | 33 +++----------- .../remote_http_trace_server.py | 6 +++ 7 files changed, 62 insertions(+), 57 deletions(-) diff --git a/weave/trace/weave_client.py b/weave/trace/weave_client.py index 5b8d3a861a9..19c2938a82a 100644 --- a/weave/trace/weave_client.py +++ b/weave/trace/weave_client.py @@ -40,12 +40,12 @@ CallUpdateReq, EndedCallSchemaForInsert, ObjCreateReq, + ObjDeleteReq, ObjectVersionFilter, ObjQueryReq, ObjReadReq, ObjSchema, ObjSchemaForInsert, - ObjsDeleteReq, Query, RefsReadBatchReq, StartedCallSchemaForInsert, @@ -753,9 +753,13 @@ def feedback( query=query, reaction=reaction, offset=offset, limit=limit ) - def delete_objects(self, object_ids: list[str]) -> None: - self.server.objs_delete( - ObjsDeleteReq(project_id=self._project_id(), object_ids=object_ids) + def delete_object(self, object: ObjectRef) -> None: + self.server.obj_delete( + ObjDeleteReq( + project_id=self._project_id(), + object_id=object.name, + digest=object.digest, + ) ) ################ Internal Helpers ################ diff --git a/weave/trace_server/clickhouse_schema.py b/weave/trace_server/clickhouse_schema.py index c203a518f30..b5c38376666 100644 --- a/weave/trace_server/clickhouse_schema.py +++ b/weave/trace_server/clickhouse_schema.py @@ -153,13 +153,16 @@ class SelectableCHObjSchema(BaseModel): is_latest: int -class ObjsDeleteCHInsertable(BaseModel): +class ObjDeleteCHInsertable(BaseModel): project_id: str object_id: str - wb_user_id: str + digest: str + kind: str + + refs: typing.List[str] = Field(default_factory=list) + val_dump: str deleted_at: datetime.datetime _project_id_v = field_validator("project_id")(validation.project_id_validator) _object_id_v = field_validator("object_id")(validation.object_id_validator) - _wb_user_id_v = field_validator("wb_user_id")(validation.wb_user_id_validator) diff --git a/weave/trace_server/clickhouse_trace_server_batched.py b/weave/trace_server/clickhouse_trace_server_batched.py index e7aa7bd86d6..a6a3d5e5a07 100644 --- a/weave/trace_server/clickhouse_trace_server_batched.py +++ b/weave/trace_server/clickhouse_trace_server_batched.py @@ -66,7 +66,7 @@ CallStartCHInsertable, CallUpdateCHInsertable, ObjCHInsertable, - ObjsDeleteCHInsertable, + ObjDeleteCHInsertable, SelectableCHCallSchema, SelectableCHObjSchema, ) @@ -616,23 +616,23 @@ def objs_query(self, req: tsi.ObjQueryReq) -> tsi.ObjQueryRes: return tsi.ObjQueryRes(objs=[_ch_obj_to_obj_schema(obj) for obj in objs]) - def objs_delete(self, req: tsi.ObjsDeleteReq) -> tsi.ObjsDeleteRes: - assert_non_null_wb_user_id(req) + def obj_delete(self, req: tsi.ObjDeleteReq) -> tsi.ObjDeleteRes: deleted_at = datetime.datetime.now() - for object_id in req.object_ids: - ch_obj = ObjsDeleteCHInsertable( - project_id=req.project_id, - wb_user_id=req.wb_user_id, - object_id=object_id, - deleted_at=deleted_at, - ) - - self._insert( - "object_versions", - data=[list(ch_obj.model_dump().values())], - column_names=list(ch_obj.model_fields.keys()), - ) - return tsi.ObjsDeleteRes() + ch_obj = ObjDeleteCHInsertable( + project_id=req.project_id, + object_id=req.object_id, + digest=req.digest, + kind="object", + deleted_at=deleted_at, + val_dump="", + refs=[], + ) + self._insert( + "object_versions", + data=[list(ch_obj.model_dump().values())], + column_names=list(ch_obj.model_fields.keys()), + ) + return tsi.ObjDeleteRes() def table_create(self, req: tsi.TableCreateReq) -> tsi.TableCreateRes: insert_rows = [] @@ -1339,14 +1339,13 @@ def _select_objs_query( PARTITION BY project_id, kind, object_id, - digest, - deleted_at - ORDER BY created_at ASC + digest + ORDER BY created_at DESC ) AS rn FROM object_versions - WHERE project_id = {{project_id: String}} AND deleted_at IS NULL + WHERE project_id = {{project_id: String}} ) - WHERE rn = 1 + WHERE rn = 1 AND deleted_at IS NULL ) WHERE project_id = {{project_id: String}} AND {conditions_part} diff --git a/weave/trace_server/external_to_internal_trace_server_adapter.py b/weave/trace_server/external_to_internal_trace_server_adapter.py index 372aa48d1b3..3331f98cd38 100644 --- a/weave/trace_server/external_to_internal_trace_server_adapter.py +++ b/weave/trace_server/external_to_internal_trace_server_adapter.py @@ -257,6 +257,10 @@ def objs_query(self, req: tsi.ObjQueryReq) -> tsi.ObjQueryRes: obj.project_id = original_project_id return res + def obj_delete(self, req: tsi.ObjDeleteReq) -> tsi.ObjDeleteRes: + req.project_id = self._idc.ext_to_int_project_id(req.project_id) + return self._ref_apply(self._internal_trace_server.obj_delete, req) + def table_create(self, req: tsi.TableCreateReq) -> tsi.TableCreateRes: req.table.project_id = self._idc.ext_to_int_project_id(req.table.project_id) return self._ref_apply(self._internal_trace_server.table_create, req) diff --git a/weave/trace_server/sqlite_trace_server.py b/weave/trace_server/sqlite_trace_server.py index a6c969a8ba3..d3867c334ae 100644 --- a/weave/trace_server/sqlite_trace_server.py +++ b/weave/trace_server/sqlite_trace_server.py @@ -672,6 +672,18 @@ def objs_query(self, req: tsi.ObjQueryReq) -> tsi.ObjQueryRes: return tsi.ObjQueryRes(objs=objs) + def obj_delete(self, req: tsi.ObjDeleteReq) -> tsi.ObjDeleteRes: + conn, cursor = get_conn_cursor(self.db_path) + with self.lock: + cursor.execute("BEGIN TRANSACTION") + for object_id in req.object_ids: + cursor.execute( + "DELETE FROM objects WHERE project_id = ? AND object_id = ?", + (req.project_id, object_id), + ) + conn.commit() + return tsi.ObjDeleteRes() + def table_create(self, req: tsi.TableCreateReq) -> tsi.TableCreateRes: conn, cursor = get_conn_cursor(self.db_path) insert_rows = [] diff --git a/weave/trace_server/trace_server_interface.py b/weave/trace_server/trace_server_interface.py index 802ba5a362f..bbe950fe956 100644 --- a/weave/trace_server/trace_server_interface.py +++ b/weave/trace_server/trace_server_interface.py @@ -375,15 +375,13 @@ class ObjQueryRes(BaseModel): objs: List[ObjSchema] -class ObjsDeleteReq(BaseModel): - # wb_user_id is automatically populated by the server - wb_user_id: Optional[str] = Field(None, description=WB_USER_ID_DESCRIPTION) - +class ObjDeleteReq(BaseModel): project_id: str - object_ids: str + object_id: str + digest: str -class ObjsDeleteRes(BaseModel): +class ObjDeleteRes(BaseModel): pass @@ -610,53 +608,32 @@ def ensure_project_exists( # Call API def call_start(self, req: CallStartReq) -> CallStartRes: ... - def call_end(self, req: CallEndReq) -> CallEndRes: ... - def call_read(self, req: CallReadReq) -> CallReadRes: ... - def calls_query(self, req: CallsQueryReq) -> CallsQueryRes: ... - def calls_query_stream(self, req: CallsQueryReq) -> Iterator[CallSchema]: ... - def calls_delete(self, req: CallsDeleteReq) -> CallsDeleteRes: ... - def calls_query_stats(self, req: CallsQueryStatsReq) -> CallsQueryStatsRes: ... - def call_update(self, req: CallUpdateReq) -> CallUpdateRes: ... # Op API def op_create(self, req: OpCreateReq) -> OpCreateRes: ... - def op_read(self, req: OpReadReq) -> OpReadRes: ... - def ops_query(self, req: OpQueryReq) -> OpQueryRes: ... # Obj API def obj_create(self, req: ObjCreateReq) -> ObjCreateRes: ... - def obj_read(self, req: ObjReadReq) -> ObjReadRes: ... - def objs_query(self, req: ObjQueryReq) -> ObjQueryRes: ... - - def objs_delete(self, req: ObjsDeleteReq) -> ObjsDeleteRes: ... - + def obj_delete(self, req: ObjDeleteReq) -> ObjDeleteRes: ... def table_create(self, req: TableCreateReq) -> TableCreateRes: ... - def table_update(self, req: TableUpdateReq) -> TableUpdateRes: ... - def table_query(self, req: TableQueryReq) -> TableQueryRes: ... - def refs_read_batch(self, req: RefsReadBatchReq) -> RefsReadBatchRes: ... - def file_create(self, req: FileCreateReq) -> FileCreateRes: ... - def file_content_read(self, req: FileContentReadReq) -> FileContentReadRes: ... - def feedback_create(self, req: FeedbackCreateReq) -> FeedbackCreateRes: ... - def feedback_query(self, req: FeedbackQueryReq) -> FeedbackQueryRes: ... - def feedback_purge(self, req: FeedbackPurgeReq) -> FeedbackPurgeRes: ... diff --git a/weave/trace_server_bindings/remote_http_trace_server.py b/weave/trace_server_bindings/remote_http_trace_server.py index 335360dfc41..d3cc8d9fbd7 100644 --- a/weave/trace_server_bindings/remote_http_trace_server.py +++ b/weave/trace_server_bindings/remote_http_trace_server.py @@ -362,6 +362,12 @@ def objs_query( "/objs/query", req, tsi.ObjQueryReq, tsi.ObjQueryRes ) + def obj_delete(self, req: tsi.ObjDeleteReq) -> tsi.ObjDeleteRes: + print("obj_delete", req) + return self._generic_request( + "/obj/delete", req, tsi.ObjDeleteReq, tsi.ObjDeleteRes + ) + def table_create( self, req: t.Union[tsi.TableCreateReq, t.Dict[str, t.Any]] ) -> tsi.TableCreateRes: From 52abf5344227373b0bed8c90a99309c722b49a53 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Thu, 5 Sep 2024 14:41:11 -0700 Subject: [PATCH 04/59] obj.delete() --- weave/trace/vals.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/weave/trace/vals.py b/weave/trace/vals.py index a54d01b0626..7d2643926ca 100644 --- a/weave/trace/vals.py +++ b/weave/trace/vals.py @@ -25,6 +25,7 @@ from weave.trace.serialize import from_json from weave.trace.table import Table from weave.trace_server.trace_server_interface import ( + ObjDeleteReq, ObjReadReq, TableQueryReq, TableRowFilter, @@ -119,6 +120,20 @@ def save(self) -> ObjectRef: raise NotImplementedError("Traceable.save not implemented") # return self.server.mutate(self.ref, mutations) + def delete(self) -> None: + if self.ref is None: + raise ValueError("Cannot delete object that is not saved") + if not isinstance(self.ref, ObjectRef): + raise ValueError("Cannot delete non-object ref") + ref: ObjectRef = typing.cast(ObjectRef, self.ref) + self.server.obj_delete( + ObjDeleteReq( + project_id=f"{ref.entity}/{ref.project}", + object_id=ref.name, + digest=ref.digest, + ) + ) + def pydantic_getattribute(self: BaseModel, name: str) -> Any: attribute = object.__getattribute__(self, name) From 29e2376a8bcfd9570ae3249707a4ced78ed6e377 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Fri, 6 Sep 2024 10:11:14 -0700 Subject: [PATCH 05/59] add test --- weave/tests/trace/test_weave_client.py | 60 +++++++++++++++++++++++ weave/trace_server/sqlite_trace_server.py | 14 ++++-- 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/weave/tests/trace/test_weave_client.py b/weave/tests/trace/test_weave_client.py index 45605cd7d06..984bf152828 100644 --- a/weave/tests/trace/test_weave_client.py +++ b/weave/tests/trace/test_weave_client.py @@ -1355,3 +1355,63 @@ def f(a): calls = list(calls) assert len(calls) == 1 assert calls[0].output["table"] == o.table.table_ref.uri() + + +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(weave_obj.ref) + with pytest.raises( + ( + weave.trace_server.sqlite_trace_server.NotFoundError, + weave.trace_server.clickhouse_trace_server_batched.NotFoundError, + ) + ): + 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.sqlite_trace_server.NotFoundError, + weave.trace_server.clickhouse_trace_server_batched.NotFoundError, + ) + ): + client.get(weave_obj3.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"], + ) + ) + assert len(versions.objs) == 2 + + # make sure we can still get the existing object versions + assert client.get(weave_obj4.ref) + assert client.get(weave_obj2.ref) + + 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 diff --git a/weave/trace_server/sqlite_trace_server.py b/weave/trace_server/sqlite_trace_server.py index d3867c334ae..b4c0cf70564 100644 --- a/weave/trace_server/sqlite_trace_server.py +++ b/weave/trace_server/sqlite_trace_server.py @@ -676,11 +676,15 @@ def obj_delete(self, req: tsi.ObjDeleteReq) -> tsi.ObjDeleteRes: conn, cursor = get_conn_cursor(self.db_path) with self.lock: cursor.execute("BEGIN TRANSACTION") - for object_id in req.object_ids: - cursor.execute( - "DELETE FROM objects WHERE project_id = ? AND object_id = ?", - (req.project_id, object_id), - ) + cursor.execute( + """ + UPDATE objects SET deleted_at = CURRENT_TIMESTAMP + WHERE project_id = ? AND + object_id = ? AND + digest = ? + """, + (req.project_id, req.object_id, req.digest), + ) conn.commit() return tsi.ObjDeleteRes() From 52839eb9dc436a41e37fddfff331044a84d8a74c Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Fri, 6 Sep 2024 11:36:32 -0700 Subject: [PATCH 06/59] wip, now with configurable include_deleted flag --- weave/tests/trace/test_weave_client.py | 38 +++++++++++++------ weave/trace/vals.py | 20 ++++++---- weave/trace_server/clickhouse_schema.py | 1 + .../clickhouse_trace_server_batched.py | 31 ++++++++++----- weave/trace_server/errors.py | 12 ++++++ weave/trace_server/sqlite_trace_server.py | 21 ++++++---- 6 files changed, 87 insertions(+), 36 deletions(-) diff --git a/weave/tests/trace/test_weave_client.py b/weave/tests/trace/test_weave_client.py index 984bf152828..1433e4e65c2 100644 --- a/weave/tests/trace/test_weave_client.py +++ b/weave/tests/trace/test_weave_client.py @@ -1364,12 +1364,7 @@ def test_object_deletion(client): assert client.get(weave_obj.ref) == obj client.delete_object(weave_obj.ref) - with pytest.raises( - ( - weave.trace_server.sqlite_trace_server.NotFoundError, - weave.trace_server.clickhouse_trace_server_batched.NotFoundError, - ) - ): + with pytest.raises(weave.trace_server.errors.ObjectDeletedError): client.get(weave_obj.ref) # create 3 versions of the object @@ -1384,12 +1379,7 @@ def test_object_deletion(client): weave_obj3.delete() # make sure we can't get the deleted object - with pytest.raises( - ( - weave.trace_server.sqlite_trace_server.NotFoundError, - weave.trace_server.clickhouse_trace_server_batched.NotFoundError, - ) - ): + with pytest.raises(weave.trace_server.errors.ObjectDeletedError): client.get(weave_obj3.ref) # count the number of versions of the object @@ -1415,3 +1405,27 @@ def test_object_deletion(client): ) ) 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): + client.get(obj1_ref) + + # 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} diff --git a/weave/trace/vals.py b/weave/trace/vals.py index 7d2643926ca..d00a18921b5 100644 --- a/weave/trace/vals.py +++ b/weave/trace/vals.py @@ -24,6 +24,7 @@ ) from weave.trace.serialize import from_json from weave.trace.table import Table +from weave.trace_server.errors import ObjectDeletedError from weave.trace_server.trace_server_interface import ( ObjDeleteReq, ObjReadReq, @@ -503,14 +504,19 @@ def make_trace_obj( if isinstance(val, ObjectRef): new_ref = val extra = val.extra - read_res = server.obj_read( - ObjReadReq( - project_id=f"{val.entity}/{val.project}", - object_id=val.name, - digest=val.digest, + try: + read_res = server.obj_read( + ObjReadReq( + project_id=f"{val.entity}/{val.project}", + object_id=val.name, + digest=val.digest, + ) ) - ) - val = from_json(read_res.obj.val, val.entity + "/" + val.project, server) + val = from_json(read_res.obj.val, val.entity + "/" + val.project, server) + except ObjectDeletedError: + # Catch error case where an object has been deleted. Val here is likely + # part of a nested object, return None to indicate a deleted object. + val = None if isinstance(val, Table): val_ref = val.ref diff --git a/weave/trace_server/clickhouse_schema.py b/weave/trace_server/clickhouse_schema.py index b5c38376666..cb08c74e791 100644 --- a/weave/trace_server/clickhouse_schema.py +++ b/weave/trace_server/clickhouse_schema.py @@ -151,6 +151,7 @@ class SelectableCHObjSchema(BaseModel): digest: str version_index: int is_latest: int + deleted_at: typing.Optional[datetime.datetime] class ObjDeleteCHInsertable(BaseModel): diff --git a/weave/trace_server/clickhouse_trace_server_batched.py b/weave/trace_server/clickhouse_trace_server_batched.py index a6a3d5e5a07..3085cc0099f 100644 --- a/weave/trace_server/clickhouse_trace_server_batched.py +++ b/weave/trace_server/clickhouse_trace_server_batched.py @@ -54,6 +54,7 @@ HardCodedFilter, combine_conditions, ) +from weave.trace_server.errors import NotFoundError, ObjectDeletedError from weave.trace_server.ids import generate_id from . import clickhouse_trace_server_migrator as wf_migrator @@ -103,10 +104,6 @@ MAX_DELETE_CALLS_COUNT = 100 -class NotFoundError(Exception): - pass - - CallCHInsertable = Union[ CallStartCHInsertable, CallEndCHInsertable, @@ -581,11 +578,21 @@ def obj_read(self, req: tsi.ObjReadReq) -> tsi.ObjReadRes: conds.append("digest = {version_digest: String}") parameters["version_digest"] = req.digest objs = self._select_objs_query( - req.project_id, conditions=conds, parameters=parameters + req.project_id, + conditions=conds, + parameters=parameters, + include_deleted=True, ) + print("objs", objs) if len(objs) == 0: raise NotFoundError(f"Obj {req.object_id}:{req.digest} not found") + # Handle deleted objects explicitly + if objs[0].deleted_at is not None: + raise ObjectDeletedError( + f"Obj {req.object_id}:{req.digest} was deleted at {objs[0].deleted_at}" + ) + return tsi.ObjReadRes(obj=_ch_obj_to_obj_schema(objs[0])) def objs_query(self, req: tsi.ObjQueryReq) -> tsi.ObjQueryRes: @@ -625,7 +632,6 @@ def obj_delete(self, req: tsi.ObjDeleteReq) -> tsi.ObjDeleteRes: kind="object", deleted_at=deleted_at, val_dump="", - refs=[], ) self._insert( "object_versions", @@ -941,6 +947,7 @@ def get_object_refs_root_val( parameters[version_param_key] = ref.version if len(conds) > 0: + conds += ["deleted_at IS NULL"] conditions = [combine_conditions(conds, "OR")] objs = self._select_objs_query( project_id=project_id_scope, @@ -1285,9 +1292,12 @@ def _select_objs_query( conditions: Optional[list[str]] = None, limit: Optional[int] = None, parameters: Optional[Dict[str, Any]] = None, + include_deleted: bool = False, ) -> list[SelectableCHObjSchema]: if not conditions: conditions = ["1 = 1"] + if not include_deleted: + conditions.append("deleted_at IS NULL") conditions_part = combine_conditions(conditions, "AND") @@ -1313,7 +1323,8 @@ def _select_objs_query( _version_index_plus_1, version_index, version_count, - is_latest + is_latest, + deleted_at FROM ( SELECT project_id, object_id, @@ -1332,7 +1343,8 @@ def _select_objs_query( ) AS _version_index_plus_1, _version_index_plus_1 - 1 AS version_index, count(*) OVER (PARTITION BY project_id, kind, object_id) as version_count, - if(_version_index_plus_1 = version_count, 1, 0) AS is_latest + if(_version_index_plus_1 = version_count, 1, 0) AS is_latest, + deleted_at FROM ( SELECT *, row_number() OVER ( @@ -1345,7 +1357,7 @@ def _select_objs_query( FROM object_versions WHERE project_id = {{project_id: String}} ) - WHERE rn = 1 AND deleted_at IS NULL + WHERE rn = 1 ) WHERE project_id = {{project_id: String}} AND {conditions_part} @@ -1373,6 +1385,7 @@ def _select_objs_query( "version_index", "version_count", "is_latest", + "deleted_at", ], row, ) diff --git a/weave/trace_server/errors.py b/weave/trace_server/errors.py index 6ebdc6dfdbb..00d0a0e2825 100644 --- a/weave/trace_server/errors.py +++ b/weave/trace_server/errors.py @@ -14,3 +14,15 @@ class InvalidRequest(Error): """Raised when a request is invalid.""" pass + + +class ObjectDeletedError(Error): + """Raised when an object has been deleted.""" + + pass + + +class NotFoundError(Error): + """Raised when not found.""" + + pass diff --git a/weave/trace_server/sqlite_trace_server.py b/weave/trace_server/sqlite_trace_server.py index b4c0cf70564..4da7b9057b4 100644 --- a/weave/trace_server/sqlite_trace_server.py +++ b/weave/trace_server/sqlite_trace_server.py @@ -13,7 +13,7 @@ from weave.trace_server import refs_internal as ri from weave.trace_server.emoji_util import detone_emojis -from weave.trace_server.errors import InvalidRequest +from weave.trace_server.errors import InvalidRequest, NotFoundError, ObjectDeletedError from weave.trace_server.feedback import ( TABLE_FEEDBACK, validate_feedback_create_req, @@ -45,10 +45,6 @@ MAX_FLUSH_AGE = 15 -class NotFoundError(Exception): - pass - - _conn_cursor: contextvars.ContextVar[ Optional[tuple[sqlite3.Connection, sqlite3.Cursor]] ] = contextvars.ContextVar("conn_cursor", default=None) @@ -642,10 +638,14 @@ def obj_read(self, req: tsi.ObjReadReq) -> tsi.ObjReadRes: objs = self._select_objs_query( req.project_id, conditions=conds, + include_deleted=True, ) if len(objs) == 0: raise NotFoundError(f"Obj {req.object_id}:{req.digest} not found") - + if objs[0].deleted_at is not None: + raise ObjectDeletedError( + f"Obj {req.object_id}:{req.digest} was deleted at {objs[0].deleted_at}" + ) return tsi.ObjReadRes(obj=objs[0]) def objs_query(self, req: tsi.ObjQueryReq) -> tsi.ObjQueryRes: @@ -1023,11 +1023,15 @@ def _select_objs_query( project_id: str, conditions: Optional[list[str]] = None, limit: Optional[int] = None, + include_deleted: bool = False, ) -> list[tsi.ObjSchema]: conn, cursor = get_conn_cursor(self.db_path) - pred = " AND ".join(conditions or ["1 = 1"]) + conditions = conditions or ["1 = 1"] + if not include_deleted: + conditions.append("deleted_at IS NULL") + pred = " AND ".join(conditions) cursor.execute( - """SELECT * FROM objects WHERE deleted_at IS NULL AND project_id = ? AND """ + """SELECT * FROM objects WHERE project_id = ? AND """ + pred + " ORDER BY created_at ASC", (project_id,), @@ -1046,6 +1050,7 @@ def _select_objs_query( digest=row[7], version_index=row[8], is_latest=row[9], + deleted_at=row[10], ) ) From ff13b4516f4b72343e24bdb89d6dec5a868a61e5 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Fri, 6 Sep 2024 11:37:35 -0700 Subject: [PATCH 07/59] wip --- weave/trace_server/clickhouse_schema.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/weave/trace_server/clickhouse_schema.py b/weave/trace_server/clickhouse_schema.py index cb08c74e791..56adf66fc75 100644 --- a/weave/trace_server/clickhouse_schema.py +++ b/weave/trace_server/clickhouse_schema.py @@ -160,9 +160,6 @@ class ObjDeleteCHInsertable(BaseModel): digest: str kind: str - refs: typing.List[str] = Field(default_factory=list) - val_dump: str - deleted_at: datetime.datetime _project_id_v = field_validator("project_id")(validation.project_id_validator) From 99fc48655bb1b5ee1451b7212bbc47a92ab02a74 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Fri, 6 Sep 2024 12:08:10 -0700 Subject: [PATCH 08/59] print --- weave/trace_server_bindings/remote_http_trace_server.py | 1 - 1 file changed, 1 deletion(-) diff --git a/weave/trace_server_bindings/remote_http_trace_server.py b/weave/trace_server_bindings/remote_http_trace_server.py index d3cc8d9fbd7..045843c2d10 100644 --- a/weave/trace_server_bindings/remote_http_trace_server.py +++ b/weave/trace_server_bindings/remote_http_trace_server.py @@ -363,7 +363,6 @@ def objs_query( ) def obj_delete(self, req: tsi.ObjDeleteReq) -> tsi.ObjDeleteRes: - print("obj_delete", req) return self._generic_request( "/obj/delete", req, tsi.ObjDeleteReq, tsi.ObjDeleteRes ) From 50c572893528f282c6c254074093a2d8f41f4b72 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Mon, 9 Sep 2024 12:09:45 -0700 Subject: [PATCH 09/59] wip --- weave/trace_server/clickhouse_schema.py | 14 ++------ .../clickhouse_trace_server_batched.py | 33 +++++++++++++------ 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/weave/trace_server/clickhouse_schema.py b/weave/trace_server/clickhouse_schema.py index 56adf66fc75..6d226b84dc6 100644 --- a/weave/trace_server/clickhouse_schema.py +++ b/weave/trace_server/clickhouse_schema.py @@ -134,6 +134,8 @@ class ObjCHInsertable(BaseModel): refs: typing.List[str] val_dump: str digest: str + deleted_at: typing.Optional[datetime.datetime] + created_at: typing.Optional[datetime.datetime] _project_id_v = field_validator("project_id")(validation.project_id_validator) _object_id_v = field_validator("object_id")(validation.object_id_validator) @@ -152,15 +154,3 @@ class SelectableCHObjSchema(BaseModel): version_index: int is_latest: int deleted_at: typing.Optional[datetime.datetime] - - -class ObjDeleteCHInsertable(BaseModel): - project_id: str - object_id: str - digest: str - kind: str - - deleted_at: datetime.datetime - - _project_id_v = field_validator("project_id")(validation.project_id_validator) - _object_id_v = field_validator("object_id")(validation.object_id_validator) diff --git a/weave/trace_server/clickhouse_trace_server_batched.py b/weave/trace_server/clickhouse_trace_server_batched.py index 3085cc0099f..da94c5623a3 100644 --- a/weave/trace_server/clickhouse_trace_server_batched.py +++ b/weave/trace_server/clickhouse_trace_server_batched.py @@ -67,7 +67,6 @@ CallStartCHInsertable, CallUpdateCHInsertable, ObjCHInsertable, - ObjDeleteCHInsertable, SelectableCHCallSchema, SelectableCHObjSchema, ) @@ -583,11 +582,9 @@ def obj_read(self, req: tsi.ObjReadReq) -> tsi.ObjReadRes: parameters=parameters, include_deleted=True, ) - print("objs", objs) if len(objs) == 0: raise NotFoundError(f"Obj {req.object_id}:{req.digest} not found") - # Handle deleted objects explicitly if objs[0].deleted_at is not None: raise ObjectDeletedError( f"Obj {req.object_id}:{req.digest} was deleted at {objs[0].deleted_at}" @@ -624,14 +621,27 @@ def objs_query(self, req: tsi.ObjQueryReq) -> tsi.ObjQueryRes: return tsi.ObjQueryRes(objs=[_ch_obj_to_obj_schema(obj) for obj in objs]) def obj_delete(self, req: tsi.ObjDeleteReq) -> tsi.ObjDeleteRes: + # To ensure no data is deleted, read obj from db first, then + # create payload with deleted_at and insert + db_obj = self.obj_read( + tsi.ObjReadReq( + project_id=req.project_id, + object_id=req.object_id, + digest=req.digest, + ) + ).obj + deleted_at = datetime.datetime.now() - ch_obj = ObjDeleteCHInsertable( + ch_obj = ObjCHInsertable( project_id=req.project_id, object_id=req.object_id, digest=req.digest, - kind="object", + kind=get_kind(db_obj.val), + val_dump=json.dumps(db_obj.val), + refs=extract_refs_from_values(db_obj.val), + base_object_class=get_base_object_class(db_obj.val), deleted_at=deleted_at, - val_dump="", + created_at=db_obj.created_at, ) self._insert( "object_versions", @@ -1339,11 +1349,12 @@ def _select_objs_query( PARTITION BY project_id, kind, object_id - ORDER BY created_at ASC + ORDER BY deleted_at DESC, created_at ASC ) AS _version_index_plus_1, _version_index_plus_1 - 1 AS version_index, count(*) OVER (PARTITION BY project_id, kind, object_id) as version_count, - if(_version_index_plus_1 = version_count, 1, 0) AS is_latest, + -- is_latest should never be true if deleted_at is not null + if(_version_index_plus_1 = version_count AND deleted_at IS NULL, 1, 0) AS is_latest, deleted_at FROM ( SELECT *, @@ -1352,10 +1363,12 @@ def _select_objs_query( kind, object_id, digest - ORDER BY created_at DESC + ORDER BY deleted_at DESC, created_at DESC ) AS rn - FROM object_versions + FROM object_versions WHERE project_id = {{project_id: String}} + -- ensures that row_number() does not count deleted versions + -- AND deleted_at IS NULL ) WHERE rn = 1 ) From 6cb4446fc8015fc14392971d45b0883b83395ceb Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Mon, 9 Sep 2024 15:29:28 -0700 Subject: [PATCH 10/59] insert with full data payload --- weave/trace_server/clickhouse_schema.py | 7 ++++-- .../clickhouse_trace_server_batched.py | 25 +++++++++---------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/weave/trace_server/clickhouse_schema.py b/weave/trace_server/clickhouse_schema.py index 6d226b84dc6..67559b8d46a 100644 --- a/weave/trace_server/clickhouse_schema.py +++ b/weave/trace_server/clickhouse_schema.py @@ -134,14 +134,17 @@ class ObjCHInsertable(BaseModel): refs: typing.List[str] val_dump: str digest: str - deleted_at: typing.Optional[datetime.datetime] - created_at: typing.Optional[datetime.datetime] _project_id_v = field_validator("project_id")(validation.project_id_validator) _object_id_v = field_validator("object_id")(validation.object_id_validator) _refs = field_validator("refs")(validation.refs_list_validator) +class ObjDeleteCHInsertable(ObjCHInsertable): + deleted_at: datetime.datetime + created_at: datetime.datetime + + class SelectableCHObjSchema(BaseModel): project_id: str object_id: str diff --git a/weave/trace_server/clickhouse_trace_server_batched.py b/weave/trace_server/clickhouse_trace_server_batched.py index da94c5623a3..71af05d12b6 100644 --- a/weave/trace_server/clickhouse_trace_server_batched.py +++ b/weave/trace_server/clickhouse_trace_server_batched.py @@ -67,6 +67,7 @@ CallStartCHInsertable, CallUpdateCHInsertable, ObjCHInsertable, + ObjDeleteCHInsertable, SelectableCHCallSchema, SelectableCHObjSchema, ) @@ -586,6 +587,7 @@ def obj_read(self, req: tsi.ObjReadReq) -> tsi.ObjReadRes: raise NotFoundError(f"Obj {req.object_id}:{req.digest} not found") if objs[0].deleted_at is not None: + # this does not get propogated to the client raise ObjectDeletedError( f"Obj {req.object_id}:{req.digest} was deleted at {objs[0].deleted_at}" ) @@ -632,7 +634,7 @@ def obj_delete(self, req: tsi.ObjDeleteReq) -> tsi.ObjDeleteRes: ).obj deleted_at = datetime.datetime.now() - ch_obj = ObjCHInsertable( + ch_obj = ObjDeleteCHInsertable( project_id=req.project_id, object_id=req.object_id, digest=req.digest, @@ -1324,6 +1326,7 @@ def _select_objs_query( project_id, object_id, created_at, + deleted_at, kind, base_object_class, refs, @@ -1333,12 +1336,12 @@ def _select_objs_query( _version_index_plus_1, version_index, version_count, - is_latest, - deleted_at + is_latest FROM ( SELECT project_id, object_id, created_at, + deleted_at, kind, base_object_class, refs, @@ -1349,13 +1352,11 @@ def _select_objs_query( PARTITION BY project_id, kind, object_id - ORDER BY deleted_at DESC, created_at ASC + ORDER BY created_at ASC ) AS _version_index_plus_1, _version_index_plus_1 - 1 AS version_index, count(*) OVER (PARTITION BY project_id, kind, object_id) as version_count, - -- is_latest should never be true if deleted_at is not null - if(_version_index_plus_1 = version_count AND deleted_at IS NULL, 1, 0) AS is_latest, - deleted_at + if(_version_index_plus_1 = version_count, 1, 0) AS is_latest FROM ( SELECT *, row_number() OVER ( @@ -1363,14 +1364,12 @@ def _select_objs_query( kind, object_id, digest - ORDER BY deleted_at DESC, created_at DESC + ORDER BY created_at DESC ) AS rn - FROM object_versions + FROM object_versions WHERE project_id = {{project_id: String}} - -- ensures that row_number() does not count deleted versions - -- AND deleted_at IS NULL ) - WHERE rn = 1 + WHERE rn = 1 AND deleted_at IS NULL ) WHERE project_id = {{project_id: String}} AND {conditions_part} @@ -1388,6 +1387,7 @@ def _select_objs_query( "project_id", "object_id", "created_at", + "deleted_at", "kind", "base_object_class", "refs", @@ -1398,7 +1398,6 @@ def _select_objs_query( "version_index", "version_count", "is_latest", - "deleted_at", ], row, ) From c8b20aa29ad4d1c5527d0b19ef97560c9d6af7de Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Mon, 9 Sep 2024 15:43:16 -0700 Subject: [PATCH 11/59] retain version # --- weave/trace_server/clickhouse_trace_server_batched.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/weave/trace_server/clickhouse_trace_server_batched.py b/weave/trace_server/clickhouse_trace_server_batched.py index 71af05d12b6..c0a423bcec6 100644 --- a/weave/trace_server/clickhouse_trace_server_batched.py +++ b/weave/trace_server/clickhouse_trace_server_batched.py @@ -1364,12 +1364,12 @@ def _select_objs_query( kind, object_id, digest - ORDER BY created_at DESC + ORDER BY created_at ASC ) AS rn FROM object_versions WHERE project_id = {{project_id: String}} ) - WHERE rn = 1 AND deleted_at IS NULL + WHERE rn = 1 ) WHERE project_id = {{project_id: String}} AND {conditions_part} From 4d3194e2c82e9f232354382371dd187a7496c7ad Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Mon, 9 Sep 2024 15:54:47 -0700 Subject: [PATCH 12/59] query magic --- weave/trace_server/clickhouse_trace_server_batched.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/weave/trace_server/clickhouse_trace_server_batched.py b/weave/trace_server/clickhouse_trace_server_batched.py index c0a423bcec6..ce8fed146ec 100644 --- a/weave/trace_server/clickhouse_trace_server_batched.py +++ b/weave/trace_server/clickhouse_trace_server_batched.py @@ -1349,14 +1349,17 @@ def _select_objs_query( digest, if (kind = 'op', 1, 0) AS is_op, row_number() OVER ( - PARTITION BY project_id, - kind, - object_id + PARTITION BY project_id, kind, object_id ORDER BY created_at ASC ) AS _version_index_plus_1, _version_index_plus_1 - 1 AS version_index, + row_number() OVER ( + PARTITION BY project_id, kind, object_id + ORDER BY (deleted_at IS NOT NULL), created_at ASC + ) AS alive_version_index, count(*) OVER (PARTITION BY project_id, kind, object_id) as version_count, - if(_version_index_plus_1 = version_count, 1, 0) AS is_latest + COUNT(*) OVER (PARTITION BY project_id, kind, object_id ORDER BY (deleted_at is not NULL)) AS deleted_count, + if(alive_version_index = (version_count - deleted_count), 1, 0) AS is_latest FROM ( SELECT *, row_number() OVER ( From 1fb0080751ead8220281274cba5617d4bfdd704b Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Mon, 9 Sep 2024 16:51:24 -0700 Subject: [PATCH 13/59] working with ops --- weave/trace/vals.py | 5 ++-- weave/trace/weave_client.py | 29 +++++++++++++------ .../clickhouse_trace_server_batched.py | 16 +++++----- 3 files changed, 32 insertions(+), 18 deletions(-) diff --git a/weave/trace/vals.py b/weave/trace/vals.py index d00a18921b5..931c88f1bda 100644 --- a/weave/trace/vals.py +++ b/weave/trace/vals.py @@ -19,6 +19,7 @@ OBJECT_ATTR_EDGE_NAME, TABLE_ROW_ID_EDGE_NAME, ObjectRef, + OpRef, RefWithExtra, TableRef, ) @@ -124,8 +125,8 @@ def save(self) -> ObjectRef: def delete(self) -> None: if self.ref is None: raise ValueError("Cannot delete object that is not saved") - if not isinstance(self.ref, ObjectRef): - raise ValueError("Cannot delete non-object ref") + if not isinstance(self.ref, (ObjectRef, OpRef)): + raise ValueError("Cannot delete refs other than objects and ops") ref: ObjectRef = typing.cast(ObjectRef, self.ref) self.server.obj_delete( ObjDeleteReq( diff --git a/weave/trace/weave_client.py b/weave/trace/weave_client.py index 98dd303cce2..2841efa751f 100644 --- a/weave/trace/weave_client.py +++ b/weave/trace/weave_client.py @@ -687,6 +687,26 @@ def delete_call(self, call: Call) -> None: ) ) + @trace_sentry.global_trace_sentry.watch() + def delete_object(self, object: ObjectRef) -> None: + self.server.obj_delete( + ObjDeleteReq( + project_id=self._project_id(), + object_id=object.name, + digest=object.digest, + ) + ) + + @trace_sentry.global_trace_sentry.watch() + def delete_op(self, op: OpRef) -> None: + self.server.obj_delete( + ObjDeleteReq( + project_id=self._project_id(), + object_id=op.name, + digest=op.digest, + ) + ) + def get_feedback( self, query: Optional[Union[Query, str]] = None, @@ -775,15 +795,6 @@ def feedback( query=query, reaction=reaction, offset=offset, limit=limit ) - def delete_object(self, object: ObjectRef) -> None: - self.server.obj_delete( - ObjDeleteReq( - project_id=self._project_id(), - object_id=object.name, - digest=object.digest, - ) - ) - def add_costs(self, costs: Dict[str, CostCreateInput]) -> CostCreateRes: """Add costs to the current project. The cost object will be created with the effective date of the date of insertion `datetime.datetime.now(ZoneInfo("UTC"))` if no effective_date is provided. diff --git a/weave/trace_server/clickhouse_trace_server_batched.py b/weave/trace_server/clickhouse_trace_server_batched.py index 22bf534c732..0ba458349c3 100644 --- a/weave/trace_server/clickhouse_trace_server_batched.py +++ b/weave/trace_server/clickhouse_trace_server_batched.py @@ -590,7 +590,7 @@ def obj_read(self, req: tsi.ObjReadReq) -> tsi.ObjReadRes: raise NotFoundError(f"Obj {req.object_id}:{req.digest} not found") if objs[0].deleted_at is not None: - # this does not get propogated to the client + # TODO: this 500's, we want the error to get propogated to the client... raise ObjectDeletedError( f"Obj {req.object_id}:{req.digest} was deleted at {objs[0].deleted_at}" ) @@ -1454,13 +1454,15 @@ def _select_objs_query( ORDER BY created_at ASC ) AS _version_index_plus_1, _version_index_plus_1 - 1 AS version_index, - row_number() OVER ( - PARTITION BY project_id, kind, object_id - ORDER BY (deleted_at IS NOT NULL), created_at ASC - ) AS alive_version_index, count(*) OVER (PARTITION BY project_id, kind, object_id) as version_count, - COUNT(*) OVER (PARTITION BY project_id, kind, object_id ORDER BY (deleted_at is not NULL)) AS deleted_count, - if(alive_version_index = (version_count - deleted_count), 1, 0) AS is_latest + CASE + WHEN ROW_NUMBER() OVER ( + PARTITION BY project_id, kind, object_id + ORDER BY created_at DESC + ) = 1 AND deleted_at IS NULL + THEN 1 + ELSE 0 + END AS is_latest FROM ( SELECT *, row_number() OVER ( From f5a9c904a2e66c70d604e6fdd68dfabb05070473 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Tue, 10 Sep 2024 11:32:02 -0700 Subject: [PATCH 14/59] wip simple object deletion fe --- .../Home/Browse3/pages/ObjectVersionPage.tsx | 223 ++++++++++++++---- .../traceServerClientTypes.ts | 6 + .../traceServerDirectClient.ts | 5 + .../wfReactInterface/tsDataModelHooks.ts | 21 ++ .../wfDataModelHooksInterface.ts | 1 + 5 files changed, 207 insertions(+), 49 deletions(-) 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 f8c85adeae7..183c5b1ea5f 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx @@ -1,9 +1,19 @@ +import { + Dialog, + DialogActions as MaterialDialogActions, + DialogContent as MaterialDialogContent, + DialogTitle as MaterialDialogTitle, +} from '@material-ui/core'; import Box from '@mui/material/Box'; +import Stack from '@mui/material/Stack'; +import {Button} from '@wandb/weave/components/Button'; import {useObjectViewEvent} from '@wandb/weave/integrations/analytics/useViewEvents'; -import React, {useMemo} from 'react'; +import React, {useMemo, useState} from 'react'; +import styled from 'styled-components'; import {maybePluralizeWord} from '../../../../../core/util/string'; import {LoadingDots} from '../../../../LoadingDots'; +import {useClosePeek} from '../context'; import {NotFoundPanel} from '../NotFoundPanel'; import {CustomWeaveTypeProjectContext} from '../typeViews/CustomWeaveTypeDispatcher'; import {WeaveCHTableSourceRefContext} from './CallPage/DataTableView'; @@ -124,60 +134,82 @@ const ObjectVersionPageInner: React.FC<{ return viewerData; }, [viewerData]); + const [deleteModalOpen, setDeleteModalOpen] = useState(false); + return ( - {objectName}{' '} - {objectVersions.loading ? ( - - ) : ( + + + - [ - - ] + {objectName}{' '} + {objectVersions.loading ? ( + + ) : ( + <> + [ + + ] + + )} - )} - - ), - Version: <>{objectVersionIndex}, - ...(baseObjectClass - ? { - Category: ( - - ), - } - : {}), - ...(refExtra - ? { - Subpath: refExtra, - } - : {}), - // 'Type Version': ( - // - // ), - }} - /> + ), + Version: <>{objectVersionIndex}, + ...(baseObjectClass + ? { + Category: ( + + ), + } + : {}), + ...(refExtra + ? { + Subpath: refExtra, + } + : {}), + // 'Type Version': ( + // + // ), + }} + /> + + + + + + + ); +}; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts index 4d6bc999d00..697e9dc7f6e 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts @@ -173,6 +173,12 @@ export type TraceObjReadRes = { obj: TraceObjSchema; }; +export type TraceObjDeleteReq = { + project_id: string; + object_id: string; + digest: string; +}; + export type TraceRefsReadBatchReq = { refs: string[]; }; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerDirectClient.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerDirectClient.ts index 4cbb917eede..6ededfbcf2d 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerDirectClient.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerDirectClient.ts @@ -34,6 +34,7 @@ import { TraceCallUpdateReq, TraceFileContentReadReq, TraceFileContentReadRes, + TraceObjDeleteReq, TraceObjQueryReq, TraceObjQueryRes, TraceObjReadReq, @@ -222,6 +223,10 @@ export class DirectTraceServerClient { return this.makeRequest('/obj/read', req); } + public objectDelete(req: TraceObjDeleteReq): Promise { + return this.makeRequest('/obj/delete', req); + } + public readBatch(req: TraceRefsReadBatchReq): Promise { return this.makeRequest( '/refs/read_batch', diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts index 754ff947065..699a0416e19 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts @@ -922,6 +922,26 @@ const useRootObjectVersions = makeTraceServerEndpointHook( res.objs.map(convertTraceServerObjectVersionToSchema) ); +const useObjectDeleteFunc = () => { + const getTsClient = useGetTraceServerClientContext(); + + const objectsDelete = useCallback( + (key: ObjectVersionKey) => { + return getTsClient().objectDelete({ + project_id: projectIdFromParts({ + entity: key.entity, + project: key.project, + }), + object_id: key.objectId, + digest: key.versionHash, + }); + }, + [getTsClient] + ); + + return objectsDelete; +}; + const useRefsReadBatch = makeTraceServerEndpointHook< 'readBatch', [string[], {skip?: boolean}?], @@ -1510,6 +1530,7 @@ export const tsWFDataModelHooks: WFDataModelHooksInterface = { useOpVersion, useOpVersions, useObjectVersion, + useObjectDeleteFunc, useRootObjectVersions, useRefsData, useApplyMutationsToRef, diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts index c8a010ffb9b..6dd891af33e 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts @@ -223,6 +223,7 @@ export type WFDataModelHooksInterface = { limit?: number, opts?: {skip?: boolean} ) => LoadableWithError; + useObjectDeleteFunc: () => (key: ObjectVersionKey) => Promise; // `useRefsData` is in beta while we integrate Shawn's new Object DB useRefsData: (refUris: string[], tableQuery?: TableQuery) => Loadable; // `useApplyMutationsToRef` is in beta while we integrate Shawn's new Object DB From 07da651f0a23a6cafb5ae94c6a5a3b34a83fdd8d Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Tue, 10 Sep 2024 12:02:20 -0700 Subject: [PATCH 15/59] refetch on delete --- .../Home/Browse3/pages/ObjectVersionPage.tsx | 3 +- .../wfReactInterface/traceServerClient.ts | 19 ++++ .../wfReactInterface/tsDataModelHooks.ts | 94 +++++++++++++------ 3 files changed, 87 insertions(+), 29 deletions(-) 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 183c5b1ea5f..53a9de5feec 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx @@ -197,7 +197,8 @@ const ObjectVersionPageInner: React.FC<{ }} /> - + + - - - - ); -}; 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 1a6e4afc577..4cca1568445 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx @@ -1,9 +1,14 @@ -import React, {useMemo} from 'react'; +import Box from '@mui/material/Box'; +import Stack from '@mui/material/Stack'; +import {Button} from '@wandb/weave/components/Button'; +import React, {useMemo, useState} from 'react'; import {LoadingDots} from '../../../../LoadingDots'; import {Tailwind} from '../../../../Tailwind'; +import {useClosePeek} from '../context'; import {NotFoundPanel} from '../NotFoundPanel'; import {OpCodeViewer} from '../OpCodeViewer'; +import {DeleteModal} from './common/DeleteModal'; import { CallsLink, opNiceName, @@ -46,10 +51,14 @@ export const OpVersionPage: React.FC<{ const OpVersionPageInner: React.FC<{ opVersion: OpVersionSchema; }> = ({opVersion}) => { - const {useOpVersions, useCallsStats} = useWFHooks(); + const {useOpVersions, useCallsStats, useObjectDeleteFunc} = useWFHooks(); const uri = opVersionKeyToRefUri(opVersion); const {entity, project, opId, versionIndex} = opVersion; + const [deleteModalOpen, setDeleteModalOpen] = useState(false); + const closePeek = useClosePeek(); + const objectDelete = useObjectDeleteFunc(); + const opVersions = useOpVersions( entity, project, @@ -75,49 +84,72 @@ const OpVersionPageInner: React.FC<{ - {opId}{' '} - {opVersions.loading ? ( - - ) : ( + + + - [ - + ) : ( + <> + [ + + ] + + )} + + ), + Version: <>{versionIndex}, + Calls: + !callsStats.loading || opVersionCallCount > 0 ? ( + - ] - - )} - - ), - Version: <>{versionIndex}, - Calls: - !callsStats.loading || opVersionCallCount > 0 ? ( - - ) : ( - <> - ), - }} - /> + ) : ( + <> + ), + }} + /> + + + + + + + + ); +}; + +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'; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts index 40dc307d573..c6c9c099a0c 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts @@ -757,10 +757,90 @@ const useOpVersion = ( }, [cachedOpVersion, key, opVersionRes, error]); }; +const useOpVersions = ( + entity: string, + project: string, + filter: OpVersionFilter, + limit?: number, + metadataOnly?: boolean, + opts?: {skip?: boolean} +): LoadableWithError => { + const getTsClient = useGetTraceServerClientContext(); + const loadingRef = useRef(false); + const [opVersionRes, setOpVersionRes] = useState< + LoadableWithError + >({ + loading: false, + error: null, + result: null, + }); + const deepFilter = useDeepMemo(filter); + + const doFetch = useCallback(() => { + if (opts?.skip) { + return; + } + setOpVersionRes({loading: true, error: null, result: null}); + loadingRef.current = true; + + const req: traceServerTypes.TraceObjQueryReq = { + project_id: projectIdFromParts({entity, project}), + filter: { + object_ids: deepFilter.opIds, + latest_only: deepFilter.latestOnly, + is_op: true, + }, + limit, + metadata_only: metadataOnly, + }; + const onSuccess = (res: traceServerTypes.TraceObjQueryRes) => { + loadingRef.current = false; + setOpVersionRes({ + loading: false, + error: null, + result: res.objs.map(convertTraceServerObjectVersionToOpSchema), + }); + }; + const onError = (e: any) => { + loadingRef.current = false; + console.error(e); + setOpVersionRes({loading: false, error: e, result: null}); + }; + getTsClient().objsQuery(req).then(onSuccess).catch(onError); + }, [ + deepFilter, + getTsClient, + opts?.skip, + entity, + project, + limit, + metadataOnly, + ]); + + useEffect(() => { + doFetch(); + }, [doFetch]); + + useEffect(() => { + return getTsClient().registerOnObjectListener(doFetch); + }, [getTsClient, doFetch]); + + return useMemo(() => { + if (opts?.skip) { + return {loading: false, error: null, result: null}; + } + if (opVersionRes == null || loadingRef.current) { + return {loading: true, error: null, result: null}; + } + return opVersionRes; + }, [opVersionRes, opts?.skip]); +}; + +// Helper function to convert trace server object version to op schema const convertTraceServerObjectVersionToOpSchema = ( obj: traceServerTypes.TraceObjSchema ): OpVersionSchema => { - const [entity, project] = obj.project_id.split('/'); + const {entity, project} = projectIdToParts(obj.project_id); return { entity, project, @@ -771,36 +851,6 @@ const convertTraceServerObjectVersionToOpSchema = ( }; }; -const useOpVersions = makeTraceServerEndpointHook< - 'objsQuery', - [string, string, OpVersionFilter, number?, boolean?, {skip?: boolean}?], - OpVersionSchema[] ->( - 'objsQuery', - ( - entity: string, - project: string, - filter: OpVersionFilter, - limit?: number, - metadataOnly?: boolean, - opts?: {skip?: boolean} - ) => ({ - params: { - project_id: projectIdFromParts({entity, project}), - filter: { - object_ids: filter.opIds, - latest_only: filter.latestOnly, - is_op: true, - }, - limit, - metadata_only: metadataOnly, - }, - skip: opts?.skip, - }), - (res): OpVersionSchema[] => - res.objs.map(convertTraceServerObjectVersionToOpSchema) -); - const useFileContent = makeTraceServerEndpointHook< 'fileContent', [string, string, string, {skip?: boolean}?], @@ -1025,13 +1075,19 @@ const useObjectDeleteFunc = () => { const getTsClient = useGetTraceServerClientContext(); const objectDelete = useCallback( - (key: ObjectVersionKey) => { + (key: ObjectVersionKey | OpVersionKey) => { + let objectId = ''; + if ('objectId' in key) { + objectId = key.objectId; + } else { + objectId = key.opId; + } return getTsClient().objectDelete({ project_id: projectIdFromParts({ entity: key.entity, project: key.project, }), - object_id: key.objectId, + object_id: objectId, digest: key.versionHash, }); }, diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts index 235e9f599e5..3c2029a76cd 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts @@ -247,7 +247,9 @@ export type WFDataModelHooksInterface = { metadataOnly?: boolean, opts?: {skip?: boolean} ) => LoadableWithError; - useObjectDeleteFunc: () => (key: ObjectVersionKey) => Promise; + useObjectDeleteFunc: () => ( + key: ObjectVersionKey | OpVersionKey + ) => Promise; // `useRefsData` is in beta while we integrate Shawn's new Object DB useRefsData: (refUris: string[], tableQuery?: TableQuery) => Loadable; // `useApplyMutationsToRef` is in beta while we integrate Shawn's new Object DB From 3d80f494aea098ae37481e07aa42a4a34657ef5f Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Wed, 30 Oct 2024 14:13:04 -0700 Subject: [PATCH 34/59] object viewer support for deleted refs --- .../Home/Browse2/SmallRef.tsx | 13 +++++++++- .../Browse3/pages/CallPage/ObjectViewer.tsx | 25 +++++++++++++++++-- .../clickhouse_trace_server_batched.py | 2 -- 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse2/SmallRef.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse2/SmallRef.tsx index 3ac3a170bcc..0e842dea81d 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse2/SmallRef.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse2/SmallRef.tsx @@ -127,7 +127,9 @@ export const SmallRef: FC<{ const objectVersion = useObjectVersion(objVersionKey); const opVersion = useOpVersion(opVersionKey); - const isDeleted = objectVersion?.error || opVersion?.error; + const isDeleted = + isObjDeleteError(objectVersion?.error) || + isObjDeleteError(opVersion?.error); const versionIndex = objectVersion.result?.versionIndex ?? opVersion.result?.versionIndex; @@ -212,3 +214,12 @@ export const parseRefMaybe = (s: string): ObjectRef | null => { return null; } }; + +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 a21e47e3917..20c8c7e2309 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, @@ -20,7 +21,7 @@ import React, { import {LoadingDots} from '../../../../../LoadingDots'; import {Browse2OpDefCode} from '../../../Browse2/Browse2OpDefCode'; -import {parseRefMaybe} from '../../../Browse2/SmallRef'; +import {objectRefDisplayName, parseRefMaybe} from '../../../Browse2/SmallRef'; import {isWeaveRef} from '../../filters/common'; import {StyledDataGrid} from '../../StyledDataGrid'; import {isCustomWeaveTypePayload} from '../../typeViews/customWeaveType.types'; @@ -151,10 +152,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); @@ -394,6 +402,19 @@ export const ObjectViewer = ({ return null; } + // Hack to show the object name with a strikethrough if deleted + if (params.row.value?._weave_is_deleted_ref) { + return ( + + {params.row.value?._weave_is_deleted_ref} + + ); + } + return ( Date: Wed, 30 Oct 2024 14:38:30 -0700 Subject: [PATCH 35/59] better hack --- .../Home/Browse3/pages/CallPage/ObjectViewer.tsx | 13 ------------- .../pages/CallPage/ObjectViewerGroupingCell.tsx | 5 ++++- 2 files changed, 4 insertions(+), 14 deletions(-) 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 20c8c7e2309..7380ae7a481 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 @@ -402,19 +402,6 @@ export const ObjectViewer = ({ return null; } - // Hack to show the object name with a strikethrough if deleted - if (params.row.value?._weave_is_deleted_ref) { - return ( - - {params.row.value?._weave_is_deleted_ref} - - ); - } - return ( - {props.value} + {deletedRef ?? props.value} } /> From 32b466e36a46483f514e6f5d96047089c2ffbaad Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Wed, 30 Oct 2024 16:07:24 -0700 Subject: [PATCH 36/59] wip recursive --- .../Home/Browse3/pages/ObjectVersionPage.tsx | 4 +- .../Home/Browse3/pages/common/DeleteModal.tsx | 47 ++++++++++-- .../traceServerClientTypes.ts | 1 + .../wfReactInterface/tsDataModelHooks.ts | 3 +- .../clickhouse_trace_server_batched.py | 75 ++++++++++++++++--- weave/trace_server/trace_server_interface.py | 6 +- 6 files changed, 112 insertions(+), 24 deletions(-) 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 bd20a5e83ba..c7513398bc6 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx @@ -292,7 +292,9 @@ const ObjectVersionPageInner: React.FC<{ open={deleteModalOpen} onClose={() => setDeleteModalOpen(false)} deleteTargetStr={`${objectVersion.objectId}:v${objectVersion.versionIndex}`} - onDelete={() => objectDelete(objectVersion)} + onDelete={(includeChildren) => + objectDelete(objectVersion, includeChildren) + } onSuccess={closePeek} /> 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 index 21fc4c51164..7afb6c85532 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx @@ -4,14 +4,16 @@ import { DialogContent as MaterialDialogContent, DialogTitle as MaterialDialogTitle, } from '@material-ui/core'; +import { Switch } from '@wandb/weave/components'; import {Button} from '@wandb/weave/components/Button'; +import { Tailwind } from '@wandb/weave/components/Tailwind'; import React, {useState} from 'react'; import styled from 'styled-components'; interface DeleteModalProps { open: boolean; onClose: () => void; - onDelete: () => Promise; + onDelete: (includeChildren: boolean) => Promise; deleteTargetStr: string; onSuccess?: () => void; } @@ -25,10 +27,11 @@ export const DeleteModal: React.FC = ({ }) => { const [deleteLoading, setDeleteLoading] = useState(false); const [error, setError] = useState(null); + const [includeChildren, setIncludeChildren] = useState(false); const handleDelete = () => { setDeleteLoading(true); - onDelete() + onDelete(includeChildren) .then(() => { onClose(); onSuccess?.(); @@ -50,16 +53,31 @@ export const DeleteModal: React.FC = ({ }} maxWidth="xs" fullWidth> + Delete {deleteTargetStr} - {error != null ? ( -

{error}

- ) : ( -

Are you sure you want to delete?

- )} - +
+ {error != null ? ( +

{error}

+ ) : ( +

Are you sure you want to delete?

+ )} +
+ {deleteTargetStr} + {/*
+ + + + +
*/} +
+ + + + +
+ ); }; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts index d5b84ef9d90..ca8a37da935 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts @@ -233,6 +233,7 @@ export type TraceObjDeleteReq = { project_id: string; object_id: string; digest: string; + include_children?: boolean; }; export type TraceRefsReadBatchReq = { diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts index c6c9c099a0c..5cb46642baa 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts @@ -1075,7 +1075,7 @@ const useObjectDeleteFunc = () => { const getTsClient = useGetTraceServerClientContext(); const objectDelete = useCallback( - (key: ObjectVersionKey | OpVersionKey) => { + (key: ObjectVersionKey | OpVersionKey, includeChildren: boolean) => { let objectId = ''; if ('objectId' in key) { objectId = key.objectId; @@ -1089,6 +1089,7 @@ const useObjectDeleteFunc = () => { }), object_id: objectId, digest: key.versionHash, + include_children: includeChildren, }); }, [getTsClient] diff --git a/weave/trace_server/clickhouse_trace_server_batched.py b/weave/trace_server/clickhouse_trace_server_batched.py index 5a51e4e3ed2..66ad5245aae 100644 --- a/weave/trace_server/clickhouse_trace_server_batched.py +++ b/weave/trace_server/clickhouse_trace_server_batched.py @@ -711,31 +711,76 @@ def obj_delete(self, req: tsi.ObjDeleteReq) -> tsi.ObjDeleteRes: # - This object should be IDENTICAL to the original, including # the created_at time, this will become the only copy of the # object when the db deduplicates on primary key - # - If purge_value is True, set val_dump to "" db_obj = self._obj_read( req.project_id, req.object_id, req.digest, - # If purging, don't read the value from the db - metadata_only=bool(req.purge_value), + metadata_only=False ) - val = "" if req.purge_value else db_obj.val_dump - ch_obj = ObjDeleteCHInsertable( + + delete_insertables: list[ObjDeleteCHInsertable] = [] + + print("DELETING OBJECT", req.project_id, db_obj.kind, db_obj.object_id, db_obj.digest) + + # if req.include_children: + if True: + obj_refs = db_obj.refs + + # for each child ref, find all objects that also reference it + for obj_ref in obj_refs: + dependents = self._select_objs_query( + req.project_id, + conditions=["has(refs, {ref: String})"], + parameters={"ref": obj_ref}, + metadata_only=True, + limit=1000, + ) + print("REF", obj_ref, "DEPENDENTS\n", dependents) + + if len(dependents) > 0: + print("CHILD HAS DEPENDENTS, SKIPPING CHILD DELETE") + continue + + # no dependents, so we can delete this object + # read from db + child_obj = ri.parse_internal_uri(obj_ref) + if not isinstance(child_obj, ri.InternalObjectRef): + raise ValueError(f"Expected InternalObjectRef, got {type(child_obj)}") + + child_ch_obj = self._obj_read( + child_obj.project_id, + child_obj.name, + child_obj.version, + metadata_only=False, + ) + delete_insertables.append(ObjDeleteCHInsertable( + project_id=child_ch_obj.project_id, + object_id=child_ch_obj.object_id, + digest=child_ch_obj.digest, + kind=child_ch_obj.kind, + val_dump=child_ch_obj.val_dump, + refs=child_ch_obj.refs, + base_object_class=child_ch_obj.base_object_class, + deleted_at=datetime.datetime.now(datetime.timezone.utc), + created_at=_ensure_datetimes_have_tz_strict(child_ch_obj.created_at), + )) + + delete_insertables.append(ObjDeleteCHInsertable( project_id=req.project_id, object_id=req.object_id, digest=req.digest, kind=db_obj.kind, - val_dump=val, + val_dump=db_obj.val_dump, refs=db_obj.refs, base_object_class=db_obj.base_object_class, deleted_at=datetime.datetime.now(datetime.timezone.utc), - # Use the original created_at time - created_at=_ensure_datetimes_have_tz(db_obj.created_at), - ) + # ! Use the original created_at time ! + created_at=_ensure_datetimes_have_tz_strict(db_obj.created_at), + )) self._insert( "object_versions", - data=[list(ch_obj.model_dump().values())], - column_names=list(ch_obj.model_fields.keys()), + data=[list(ch_obj.model_dump().values()) for ch_obj in delete_insertables], + column_names=list(delete_insertables[0].model_fields.keys()), ) return tsi.ObjDeleteRes() @@ -1948,6 +1993,14 @@ def _ensure_datetimes_have_tz( return dt.replace(tzinfo=datetime.timezone.utc) return dt +def _ensure_datetimes_have_tz_strict( + dt: datetime.datetime, +) -> datetime.datetime: + res = _ensure_datetimes_have_tz(dt) + if res is None: + raise ValueError(f"Datetime is None: {dt}") + return res + def _nullable_dict_dump_to_dict( val: Optional[str], diff --git a/weave/trace_server/trace_server_interface.py b/weave/trace_server/trace_server_interface.py index b16ffce70bf..3021e61c92b 100644 --- a/weave/trace_server/trace_server_interface.py +++ b/weave/trace_server/trace_server_interface.py @@ -470,10 +470,10 @@ class ObjDeleteReq(BaseModel): object_id: str digest: str - purge_value: Optional[bool] = Field( + include_children: Optional[bool] = Field( default=False, - description="If true, the object value will be permanently deleted " - "from the database. This action is irreversible.", + description="Recursively delete children of this object that are not " + "used by other objects" ) From 38c4074e293fb17013a532fb1c5d82c1134c4e01 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Thu, 7 Nov 2024 10:15:14 -0800 Subject: [PATCH 37/59] simpler --- .../clickhouse_trace_server_batched.py | 101 ++++++++---------- 1 file changed, 47 insertions(+), 54 deletions(-) diff --git a/weave/trace_server/clickhouse_trace_server_batched.py b/weave/trace_server/clickhouse_trace_server_batched.py index 66ad5245aae..d660c0b17e1 100644 --- a/weave/trace_server/clickhouse_trace_server_batched.py +++ b/weave/trace_server/clickhouse_trace_server_batched.py @@ -30,17 +30,7 @@ import threading from collections import defaultdict from contextlib import contextmanager -from typing import ( - Any, - Dict, - Iterator, - Optional, - Sequence, - Set, - Tuple, - Union, - cast, -) +from typing import Any, Dict, Iterator, Optional, Sequence, Set, Tuple, Union, cast from zoneinfo import ZoneInfo import clickhouse_connect @@ -706,24 +696,21 @@ def objs_query(self, req: tsi.ObjQueryReq) -> tsi.ObjQueryRes: def obj_delete(self, req: tsi.ObjDeleteReq) -> tsi.ObjDeleteRes: # 1. Read the entire object from the db based on id and digest - # 2. Set deleted_at to the current time - # 3. Insert this object into object_versions - # - This object should be IDENTICAL to the original, including - # the created_at time, this will become the only copy of the - # object when the db deduplicates on primary key + # 2. Check if object has any children refs + # 3. For each child ref, find all objects that also reference it + # 4. If any dependents are found, skip this object, else add to delete list + # 5. For all in delete list: + # - Set deleted_at to the current time + # - Insert this object into object_versions + # - This object should be IDENTICAL to the original, including + # the created_at time, this will become the only copy of the + # object when the db deduplicates on primary key db_obj = self._obj_read( - req.project_id, - req.object_id, - req.digest, - metadata_only=False + req.project_id, req.object_id, req.digest, metadata_only=False ) - delete_insertables: list[ObjDeleteCHInsertable] = [] - print("DELETING OBJECT", req.project_id, db_obj.kind, db_obj.object_id, db_obj.digest) - - # if req.include_children: - if True: + if req.include_children: obj_refs = db_obj.refs # for each child ref, find all objects that also reference it @@ -735,48 +722,53 @@ def obj_delete(self, req: tsi.ObjDeleteReq) -> tsi.ObjDeleteRes: metadata_only=True, limit=1000, ) - print("REF", obj_ref, "DEPENDENTS\n", dependents) if len(dependents) > 0: - print("CHILD HAS DEPENDENTS, SKIPPING CHILD DELETE") continue # no dependents, so we can delete this object - # read from db child_obj = ri.parse_internal_uri(obj_ref) if not isinstance(child_obj, ri.InternalObjectRef): - raise ValueError(f"Expected InternalObjectRef, got {type(child_obj)}") - + raise ValueError( + f"Expected InternalObjectRef, got {type(child_obj)}" + ) + child_ch_obj = self._obj_read( child_obj.project_id, child_obj.name, child_obj.version, metadata_only=False, ) - delete_insertables.append(ObjDeleteCHInsertable( - project_id=child_ch_obj.project_id, - object_id=child_ch_obj.object_id, - digest=child_ch_obj.digest, - kind=child_ch_obj.kind, - val_dump=child_ch_obj.val_dump, - refs=child_ch_obj.refs, - base_object_class=child_ch_obj.base_object_class, - deleted_at=datetime.datetime.now(datetime.timezone.utc), - created_at=_ensure_datetimes_have_tz_strict(child_ch_obj.created_at), - )) - - delete_insertables.append(ObjDeleteCHInsertable( - project_id=req.project_id, - object_id=req.object_id, - digest=req.digest, - kind=db_obj.kind, - val_dump=db_obj.val_dump, - refs=db_obj.refs, - base_object_class=db_obj.base_object_class, - deleted_at=datetime.datetime.now(datetime.timezone.utc), - # ! Use the original created_at time ! - created_at=_ensure_datetimes_have_tz_strict(db_obj.created_at), - )) + delete_insertables.append( + ObjDeleteCHInsertable( + project_id=child_ch_obj.project_id, + object_id=child_ch_obj.object_id, + digest=child_ch_obj.digest, + kind=child_ch_obj.kind, + val_dump=child_ch_obj.val_dump, + refs=child_ch_obj.refs, + base_object_class=child_ch_obj.base_object_class, + deleted_at=datetime.datetime.now(datetime.timezone.utc), + created_at=_ensure_datetimes_have_tz_strict( + child_ch_obj.created_at + ), + ) + ) + + delete_insertables.append( + ObjDeleteCHInsertable( + project_id=req.project_id, + object_id=req.object_id, + digest=req.digest, + kind=db_obj.kind, + val_dump=db_obj.val_dump, + refs=db_obj.refs, + base_object_class=db_obj.base_object_class, + deleted_at=datetime.datetime.now(datetime.timezone.utc), + # ! Use the original created_at time ! + created_at=_ensure_datetimes_have_tz_strict(db_obj.created_at), + ) + ) self._insert( "object_versions", data=[list(ch_obj.model_dump().values()) for ch_obj in delete_insertables], @@ -1993,6 +1985,7 @@ def _ensure_datetimes_have_tz( return dt.replace(tzinfo=datetime.timezone.utc) return dt + def _ensure_datetimes_have_tz_strict( dt: datetime.datetime, ) -> datetime.datetime: From c1629fa5b3d1479555b536143e02aa3d21f085ac Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Tue, 26 Nov 2024 12:32:34 -0800 Subject: [PATCH 38/59] fix --- weave/trace_server/clickhouse_schema.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/weave/trace_server/clickhouse_schema.py b/weave/trace_server/clickhouse_schema.py index 45a16c64bc5..05deb8b1d61 100644 --- a/weave/trace_server/clickhouse_schema.py +++ b/weave/trace_server/clickhouse_schema.py @@ -152,4 +152,4 @@ class SelectableCHObjSchema(BaseModel): digest: str version_index: int is_latest: int - deleted_at: typing.Optional[datetime.datetime] + deleted_at: Optional[datetime.datetime] From 8ae0003687c322466e7c5881e0e80d0f76375cb8 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Tue, 26 Nov 2024 12:58:25 -0800 Subject: [PATCH 39/59] fixes, working --- .../Home/Browse3/pages/ObjectVersionPage.tsx | 50 ++++++------ .../Home/Browse3/pages/common/DeleteModal.tsx | 78 ++++++++----------- .../clickhouse_trace_server_batched.py | 6 +- 3 files changed, 59 insertions(+), 75 deletions(-) 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 ad6d4fea429..b4ace686651 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx @@ -1,21 +1,21 @@ import Box from '@mui/material/Box'; import Stack from '@mui/material/Stack'; -import { Button } from '@wandb/weave/components/Button'; -import { useObjectViewEvent } from '@wandb/weave/integrations/analytics/useViewEvents'; -import React, { useMemo, useState } from 'react'; +import {Button} from '@wandb/weave/components/Button'; +import {useObjectViewEvent} from '@wandb/weave/integrations/analytics/useViewEvents'; +import React, {useMemo, useState} from 'react'; -import { maybePluralizeWord } from '../../../../../core/util/string'; -import { Icon, IconName } from '../../../../Icon'; -import { LoadingDots } from '../../../../LoadingDots'; -import { Tailwind } from '../../../../Tailwind'; -import { Tooltip } from '../../../../Tooltip'; -import { useClosePeek } from '../context'; -import { NotFoundPanel } from '../NotFoundPanel'; -import { CustomWeaveTypeProjectContext } from '../typeViews/CustomWeaveTypeDispatcher'; -import { WeaveCHTableSourceRefContext } from './CallPage/DataTableView'; -import { ObjectViewerSection } from './CallPage/ObjectViewerSection'; -import { WFHighLevelCallFilter } from './CallsPage/callsTableFilter'; -import { DeleteModal } from './common/DeleteModal'; +import {maybePluralizeWord} from '../../../../../core/util/string'; +import {Icon, IconName} from '../../../../Icon'; +import {LoadingDots} from '../../../../LoadingDots'; +import {Tailwind} from '../../../../Tailwind'; +import {Tooltip} from '../../../../Tooltip'; +import {useClosePeek} from '../context'; +import {NotFoundPanel} from '../NotFoundPanel'; +import {CustomWeaveTypeProjectContext} from '../typeViews/CustomWeaveTypeDispatcher'; +import {WeaveCHTableSourceRefContext} from './CallPage/DataTableView'; +import {ObjectViewerSection} from './CallPage/ObjectViewerSection'; +import {WFHighLevelCallFilter} from './CallsPage/callsTableFilter'; +import {DeleteModal} from './common/DeleteModal'; import { CallLink, CallsLink, @@ -23,20 +23,20 @@ import { objectVersionText, OpVersionLink, } from './common/Links'; -import { CenteredAnimatedLoader } from './common/Loader'; +import {CenteredAnimatedLoader} from './common/Loader'; import { ScrollableTabContent, SimpleKeyValueTable, SimplePageLayoutWithHeader, } from './common/SimplePageLayout'; -import { EvaluationLeaderboardTab } from './LeaderboardTab'; -import { TabPrompt } from './TabPrompt'; -import { TabUseDataset } from './TabUseDataset'; -import { TabUseModel } from './TabUseModel'; -import { TabUseObject } from './TabUseObject'; -import { TabUsePrompt } from './TabUsePrompt'; -import { KNOWN_BASE_OBJECT_CLASSES } from './wfReactInterface/constants'; -import { useWFHooks } from './wfReactInterface/context'; +import {EvaluationLeaderboardTab} from './LeaderboardTab'; +import {TabPrompt} from './TabPrompt'; +import {TabUseDataset} from './TabUseDataset'; +import {TabUseModel} from './TabUseModel'; +import {TabUseObject} from './TabUseObject'; +import {TabUsePrompt} from './TabUsePrompt'; +import {KNOWN_BASE_OBJECT_CLASSES} from './wfReactInterface/constants'; +import {useWFHooks} from './wfReactInterface/context'; import { objectVersionKeyToRefUri, refUriToOpVersionKey, @@ -281,7 +281,7 @@ const ObjectVersionPageInner: React.FC<{ open={deleteModalOpen} onClose={() => setDeleteModalOpen(false)} deleteTargetStr={`${objectVersion.objectId}:v${objectVersion.versionIndex}`} - onDelete={objectDelete(objectVersion)} + onDelete={() => objectDelete(objectVersion)} onSuccess={closePeek} /> 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 index e8df59aee66..1650db545b2 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx @@ -4,10 +4,9 @@ import { DialogContent as MaterialDialogContent, DialogTitle as MaterialDialogTitle, } from '@material-ui/core'; -import { Switch } from '@wandb/weave/components'; -import { Button } from '@wandb/weave/components/Button'; -import { Tailwind } from '@wandb/weave/components/Tailwind'; -import React, { useState } from 'react'; +import {Button} from '@wandb/weave/components/Button'; +import {Tailwind} from '@wandb/weave/components/Tailwind'; +import React, {useState} from 'react'; import styled from 'styled-components'; interface DeleteModalProps { @@ -27,7 +26,6 @@ export const DeleteModal: React.FC = ({ }) => { const [deleteLoading, setDeleteLoading] = useState(false); const [error, setError] = useState(null); - const [includeChildren, setIncludeChildren] = useState(false); const handleDelete = () => { setDeleteLoading(true); @@ -54,48 +52,34 @@ export const DeleteModal: React.FC = ({ maxWidth="xs" fullWidth> - Delete {deleteTargetStr} - -
- {error != null ? ( -

{error}

- ) : ( -

Are you sure you want to delete?

- )} -
- - {deleteTargetStr} - - - - - -
- - - - -
- + Delete {deleteTargetStr} + +
+ {error != null ? ( +

{error}

+ ) : ( +

Are you sure you want to delete?

+ )} +
+ {deleteTargetStr} + + + + + ); diff --git a/weave/trace_server/clickhouse_trace_server_batched.py b/weave/trace_server/clickhouse_trace_server_batched.py index ee1c9bef445..40d2519b793 100644 --- a/weave/trace_server/clickhouse_trace_server_batched.py +++ b/weave/trace_server/clickhouse_trace_server_batched.py @@ -604,10 +604,10 @@ def obj_create(self, req: tsi.ObjCreateReq) -> tsi.ObjCreateRes: @staticmethod def _make_conds_from_digest( digest: str, - ) -> tuple[list[str], Dict[str, Union[str, int]]]: + ) -> tuple[list[str], dict[str, Union[str, int]]]: (is_version, version_index) = digest_is_version_like(digest) conds: list[str] = [] - parameters: Dict[str, Union[str, int]] = {} + parameters: dict[str, Union[str, int]] = {} if digest == "latest": conds.append("is_latest = 1") else: @@ -630,7 +630,7 @@ def _obj_read( ) -> SelectableCHObjSchema: (is_version, version_index) = digest_is_version_like(digest) conds: list[str] = [] - parameters: Dict[str, Union[str, int]] = {"object_id": object_id} + parameters: dict[str, Union[str, int]] = {"object_id": object_id} if digest == "latest": conds.append("is_latest = 1") else: From d3c63f9378c1138f9d38f1cbeb09dde67cd5441c Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Tue, 3 Dec 2024 09:12:24 -0800 Subject: [PATCH 40/59] costmetic code refactor --- .../Browse3/pages/CallPage/ObjectViewer.tsx | 2 +- .../CallPage/ObjectViewerGroupingCell.tsx | 4 +- .../Home/Browse3/pages/ObjectVersionPage.tsx | 26 ++--------- .../Home/Browse3/pages/OpVersionPage.tsx | 26 ++--------- .../Home/Browse3/pages/common/DeleteModal.tsx | 46 +++++++++++++++++++ 5 files changed, 57 insertions(+), 47 deletions(-) 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 384ed520e0b..093aea893df 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 @@ -22,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 {isCustomWeaveTypePayload} from '../../typeViews/customWeaveType.types'; @@ -48,7 +49,6 @@ import { TraverseContext, } from './traverse'; import {ValueView} from './ValueView'; -import {objectRefDisplayName} from '../../../Browse2/SmallRef'; type Data = Record | any[]; 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 094cf8045cc..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 @@ -33,8 +33,8 @@ export const ObjectViewerGroupingCell: FC< }; const deletedRef = row.value?._weave_is_deleted_ref; - const tooltipContent = row.path.toString(); + const textContent = deletedRef ?? props.value; const box = ( - {deletedRef ?? 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 b4ace686651..fa188bfa0b8 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx @@ -1,21 +1,19 @@ import Box from '@mui/material/Box'; import Stack from '@mui/material/Stack'; -import {Button} from '@wandb/weave/components/Button'; import {useObjectViewEvent} from '@wandb/weave/integrations/analytics/useViewEvents'; -import React, {useMemo, useState} from 'react'; +import React, {useMemo} from 'react'; import {maybePluralizeWord} from '../../../../../core/util/string'; import {Icon, IconName} from '../../../../Icon'; import {LoadingDots} from '../../../../LoadingDots'; import {Tailwind} from '../../../../Tailwind'; import {Tooltip} from '../../../../Tooltip'; -import {useClosePeek} from '../context'; import {NotFoundPanel} from '../NotFoundPanel'; import {CustomWeaveTypeProjectContext} from '../typeViews/CustomWeaveTypeDispatcher'; import {WeaveCHTableSourceRefContext} from './CallPage/DataTableView'; import {ObjectViewerSection} from './CallPage/ObjectViewerSection'; import {WFHighLevelCallFilter} from './CallsPage/callsTableFilter'; -import {DeleteModal} from './common/DeleteModal'; +import {DeleteObjectButtonWithModal} from './common/DeleteModal'; import { CallLink, CallsLink, @@ -114,10 +112,7 @@ const ObjectVersionPageInner: React.FC<{ }> = ({objectVersion}) => { useObjectViewEvent(objectVersion); - const closePeek = useClosePeek(); - const {useRootObjectVersions, useCalls, useRefsData, useObjectDeleteFunc} = - useWFHooks(); - const objectDelete = useObjectDeleteFunc(); + const {useRootObjectVersions, useCalls, useRefsData} = useWFHooks(); const entityName = objectVersion.entity; const projectName = objectVersion.project; const objectName = objectVersion.objectId; @@ -198,7 +193,6 @@ const ObjectVersionPageInner: React.FC<{ return viewerData; }, [viewerData]); - const [deleteModalOpen, setDeleteModalOpen] = useState(false); const isDataset = baseObjectClass === 'Dataset' && refExtra == null; const isEvaluation = baseObjectClass === 'Evaluation' && refExtra == null; const evalHasCalls = (consumingCalls.result?.length ?? 0) > 0; @@ -269,21 +263,9 @@ const ObjectVersionPageInner: React.FC<{ }} /> - -