Skip to content

Commit

Permalink
[BUGFIX] server: Skip responses without user ids when indexing (#5093)
Browse files Browse the repository at this point in the history
# Pull Request Template
<!-- Please include a summary of the changes and the related issue.
Please also include relevant motivation and context. List any
dependencies that are required for this change. -->

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**
<!-- Please delete options that are not relevant. Remember to title the
PR according to the type of change -->

- Bug fix (non-breaking change which fixes an issue)

**How Has This Been Tested**
<!-- Please add some reference about how your feature has been tested.
-->

**Checklist**
<!-- Please go over the list and make sure you've taken everything into
account -->

- 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/)
  • Loading branch information
frascuchon authored Jun 25, 2024
1 parent a5c13fc commit 1361bf1
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 0 deletions.
4 changes: 4 additions & 0 deletions argilla-server/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions argilla-server/src/argilla_server/search_engine/commons.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions argilla-server/tests/unit/search_engine/test_commons.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 1361bf1

Please sign in to comment.