Skip to content

Commit

Permalink
fix: delete records feature (#1737)
Browse files Browse the repository at this point in the history
Some problems detected:

Change from httpx.delete to httpx.request with method="DELETE", since the delete method does not accept body param (data or json)
Fix response data when no records match for deletion
Refs #1714
  • Loading branch information
frascuchon committed Sep 30, 2022
1 parent 030e20d commit d43fe87
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 26 deletions.
2 changes: 1 addition & 1 deletion docs/reference/python/python_client.rst
Expand Up @@ -15,7 +15,7 @@ Methods
-------

.. automodule:: rubrix
:members: init, log, load, copy, delete, set_workspace, get_workspace
:members: init, log, load, copy, delete, set_workspace, get_workspace, delete_records

.. _python ref records:

Expand Down
27 changes: 9 additions & 18 deletions src/rubrix/client/api.py
Expand Up @@ -422,36 +422,27 @@ def delete_records(
) -> Tuple[int, int]:
"""Delete records from a Rubrix dataset.
Parameters:
-----------
name:
The dataset name.
query:
An ElasticSearch query with the
`query string syntax <https://rubrix.readthedocs.io/en/stable/guides/queries.html>`_
ids:
If provided, deletes dataset records with given ids.
discard_only:
If `True`, matched records won't be deleted. Instead, they will be marked
as `Discarded`
discard_when_forbidden:
Only super-user or dataset creator can delete records from a dataset.
Args:
name: The dataset name.
query: An ElasticSearch query with the `query string syntax
<https://rubrix.readthedocs.io/en/stable/guides/queries.html>`_
ids: If provided, deletes dataset records with given ids.
discard_only: If `True`, matched records won't be deleted. Instead, they will be marked as `Discarded`
discard_when_forbidden: Only super-user or dataset creator can delete records from a dataset.
So, running "hard" deletion for other users will raise an `ForbiddenApiError` error.
If this parameter is `True`, the client API will automatically try to mark as ``Discarded``
records instead. Default, `True`
Returns:
--------
The total of matched records and real number of processed errors. These numbers could not
be the same if some data conflicts are found during operations (some matched records change during
deletion).
Examples:
**Delete by id**:
>>> ## Delete by id
>>> import rubrix as rb
>>> rb.delete_records(name="example-dataset", ids=[1,3,5])
**Discard records by query**:
>>> ## Discard records by query
>>> import rubrix as rb
>>> rb.delete_records(name="example-dataset", query="metadata.code=33", discard_only=True)
"""
Expand Down
3 changes: 2 additions & 1 deletion src/rubrix/client/sdk/client.py
Expand Up @@ -123,7 +123,8 @@ def put(self, path: str, *args, **kwargs):
@with_httpx_error_handler
def delete(self, path: str, *args, **kwargs):
path = self._normalize_path(path)
response = self.__httpx__.delete(
response = self.__httpx__.request(
method="DELETE",
url=path,
headers=self.get_headers(),
*args,
Expand Down
12 changes: 6 additions & 6 deletions src/rubrix/server/services/storage/service.py
Expand Up @@ -30,9 +30,9 @@

@dataclasses.dataclass
class DeleteRecordsOut:
processed: int
discarded: Optional[int] = None
deleted: Optional[int] = None
processed: int = 0
discarded: int = 0
deleted: int = 0


class RecordsStorageService:
Expand Down Expand Up @@ -97,7 +97,7 @@ async def delete_records(
)

return DeleteRecordsOut(
processed=processed,
discarded=discarded,
deleted=deleted,
processed=processed or 0,
discarded=discarded or 0,
deleted=deleted or 0,
)
Expand Up @@ -89,3 +89,22 @@ def test_delete_records_without_permission(mocked_client):
assert matched, processed == (1, 1)
finally:
mocked_client.reset_default_user()


def test_delete_records_with_unmatched_records(mocked_client):
dataset = "test_delete_records_with_unmatched_records"
import rubrix as rb

rb.delete(dataset)
rb.log(
name=dataset,
records=[
rb.TextClassificationRecord(
id=i, text="This is the text", metadata=dict(idx=i)
)
for i in range(0, 50)
],
)

matched, processed = rb.delete_records(dataset, ids=["you-wont-find-me-here"])
assert (matched, processed) == (0, 0)
5 changes: 5 additions & 0 deletions tests/helpers.py
Expand Up @@ -80,6 +80,11 @@ def delete(self, *args, **kwargs):
headers = {**self._header, **request_headers}
return self._client.delete(*args, headers=headers, **kwargs)

def request(self, *args, **kwargs):
request_headers = kwargs.pop("headers", {})
headers = {**self._header, **request_headers}
return self._client.request(*args, headers=headers, **kwargs)

def post(self, *args, **kwargs):
request_headers = kwargs.pop("headers", {})
headers = {**self._header, **request_headers}
Expand Down

0 comments on commit d43fe87

Please sign in to comment.