From 1361bf1a523729bb185e1fa8538df9cf95d71c17 Mon Sep 17 00:00:00 2001 From: Paco Aranda Date: Tue, 25 Jun 2024 09:38:50 +0200 Subject: [PATCH] [BUGFIX] server: Skip responses without user ids when indexing (#5093) # Pull Request Template Since the `user_id` is a nullable column, the search engine must tackle responses for those scenarios. This PR changes the search engine and skips responses when the user_id is None. This can be tackled in a better way in the future once we have a clearer view of how to deal with deleted users. Refs: #5070 **Type of change** - Bug fix (non-breaking change which fixes an issue) **How Has This Been Tested** **Checklist** - I added relevant documentation - follows the style guidelines of this project - I did a self-review of my code - I made corresponding changes to the documentation - I confirm My changes generate no new warnings - I have added tests that prove my fix is effective or that my feature works - I have added relevant notes to the CHANGELOG.md file (See https://keepachangelog.com/) --- argilla-server/CHANGELOG.md | 4 ++++ .../argilla_server/search_engine/commons.py | 1 + .../tests/unit/search_engine/test_commons.py | 20 +++++++++++++++++++ 3 files changed, 25 insertions(+) diff --git a/argilla-server/CHANGELOG.md b/argilla-server/CHANGELOG.md index a665acf0e3..3d24117370 100644 --- a/argilla-server/CHANGELOG.md +++ b/argilla-server/CHANGELOG.md @@ -16,6 +16,10 @@ These are the section headers that we use: ## [Unreleased]() +### Fixed + +- Fixed error when indexing responses without user_id. ([#5093](https://github.com/argilla-io/argilla/pull/5093)) + ## [2.0.0rc1](https://github.com/argilla-io/argilla/compare/v1.29.0...v2.0.0rc1) ### Removed diff --git a/argilla-server/src/argilla_server/search_engine/commons.py b/argilla-server/src/argilla_server/search_engine/commons.py index 2030b59ae5..c9a3161ed9 100644 --- a/argilla-server/src/argilla_server/search_engine/commons.py +++ b/argilla-server/src/argilla_server/search_engine/commons.py @@ -575,6 +575,7 @@ def _map_record_responses_to_es(responses: List[Response]) -> Dict[str, Any]: "status": response.status, } for response in responses + if response.user is not None } @staticmethod diff --git a/argilla-server/tests/unit/search_engine/test_commons.py b/argilla-server/tests/unit/search_engine/test_commons.py index ecba3232a6..b1ad061cc1 100644 --- a/argilla-server/tests/unit/search_engine/test_commons.py +++ b/argilla-server/tests/unit/search_engine/test_commons.py @@ -1100,6 +1100,26 @@ async def test_update_record_response( }, } + async def test_update_record_response_without_user_id( + self, + search_engine: BaseElasticAndOpenSearchEngine, + opensearch: OpenSearch, + test_banking_sentiment_dataset: Dataset, + ): + record = test_banking_sentiment_dataset.records[0] + question = test_banking_sentiment_dataset.questions[0] + + response = await ResponseFactory.create(record=record, values={question.name: {"value": "test"}}, user=None) + record = await response.awaitable_attrs.record + await record.awaitable_attrs.dataset + await search_engine.update_record_response(response) + + index_name = es_index_name_for_dataset(test_banking_sentiment_dataset) + + results = opensearch.get(index=index_name, id=record.id) + + assert results["_source"]["responses"] == {} + @pytest.mark.parametrize("annotators_size", [20, 200, 400]) async def test_annotators_limits( self,