Skip to content

Python: fix(redis): wrap IndexDefinition prefix in list; use _get_redis_key in JSON delete #13970

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nileshpatil6 wants to merge 3 commits into
base: main
Choose a base branch
from
+32 −4
Open

Python: fix(redis): wrap IndexDefinition prefix in list; use _get_redis_key in JSON delete #13970

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
71226fe
fix(redis): wrap IndexDefinition prefix in list and use _get_redis_ke…
nileshpatil6 May 8, 2026
1f578ac
fix(redis): conditionally apply IndexDefinition prefix based on prefi…
nileshpatil6 May 8, 2026
2013d1f
test(redis): add coverage for prefix-aware IndexDefinition and JSON d…
nileshpatil6 May 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions python/semantic_kernel/connectors/redis.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,9 +277,12 @@ async def ensure_collection_exists(self, **kwargs) -> None:
return
raise VectorStoreOperationException("Invalid index type supplied.")
fields = _definition_to_redis_fields(self.definition, self.collection_type)
index_definition = IndexDefinition(
prefix=f"{self.collection_name}:", index_type=INDEX_TYPE_MAP[self.collection_type]
)
if self.prefix_collection_name_to_key_names:
index_definition = IndexDefinition(
prefix=[f"{self.collection_name}:"], index_type=INDEX_TYPE_MAP[self.collection_type]
)
else:
index_definition = IndexDefinition(index_type=INDEX_TYPE_MAP[self.collection_type])
await self.redis_database.ft(self.collection_name).create_index(fields, definition=index_definition, **kwargs)
Comment on lines 279 to 286

@override
Expand Down Expand Up @@ -706,7 +709,7 @@ def _add_key(self, key: TKey, record: dict[str, Any]) -> dict[str, Any]:

@override
async def _inner_delete(self, keys: Sequence[str], **kwargs: Any) -> None:
await asyncio.gather(*[self.redis_database.json().delete(key, **kwargs) for key in keys])
await asyncio.gather(*[self.redis_database.json().delete(self._get_redis_key(key), **kwargs) for key in keys])
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test coverage. This fix is correct — RedisHashsetCollection._inner_delete (line 581) already uses _get_redis_key. However, the existing test_delete (test_redis_store.py:274) only uses non-prefix collections (collection_hash/collection_json), where _get_redis_key returns the key unchanged. The test passes identically with the old and new code. Please add a test with collection_with_prefix_json that asserts json().delete() receives 'test:id1', not 'id1'.


@override
def _serialize_dicts_to_store_models(
Expand Down
25 changes: 25 additions & 0 deletions python/tests/unit/connectors/memory/test_redis_store.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,3 +306,28 @@ async def test_create_index_manual(collection_hash, mock_ensure_collection_exist
async def test_create_index_fail(collection_hash, mock_ensure_collection_exists):
with raises(VectorStoreOperationException, match="Invalid index type supplied."):
await collection_hash.ensure_collection_exists(index_definition="index_definition", fields="fields")


async def test_create_index_respects_prefix_flag(
collection_hash, collection_with_prefix_hash, mock_ensure_collection_exists
):
from redis.commands.search.index_definition import IndexDefinition

# Without prefix flag: IndexDefinition should NOT contain the collection prefix
await collection_hash.ensure_collection_exists()
index_def = mock_ensure_collection_exists.call_args.kwargs["definition"]
assert isinstance(index_def, IndexDefinition)
assert "test:" not in index_def.args

mock_ensure_collection_exists.reset_mock()

# With prefix flag: IndexDefinition should include ["test:"] as prefix
await collection_with_prefix_hash.ensure_collection_exists()
index_def = mock_ensure_collection_exists.call_args.kwargs["definition"]
assert isinstance(index_def, IndexDefinition)
assert "test:" in index_def.args


async def test_delete_json_with_prefix(collection_with_prefix_json, mock_delete_json):
await collection_with_prefix_json._inner_delete(["id1"])
mock_delete_json.assert_called_once_with("test:id1")
Loading