Skip to content

Commit 0675d55

Browse files
fix: address CodeRabbit PR feedback for OpenSearch store
- Fix type annotation: AsyncElasticsearch → AsyncOpenSearch in async test - Add key/collection/version fields to serialized documents (following PR #204) - Update test snapshots to expect version, key, and collection fields - Fix ImportError message in sync store to reference correct package - Run codegen to regenerate sync library with fixes Co-authored-by: William Easton <strawgate@users.noreply.github.com>
1 parent 7930314 commit 0675d55

File tree

4 files changed

+30
-11
lines changed

4 files changed

+30
-11
lines changed

key-value/key-value-aio/src/key_value/aio/stores/opensearch/store.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ async def _put_managed_entry(
359359
index_name: str = self._get_index_name(collection=collection)
360360
document_id: str = self._get_document_id(key=key)
361361

362-
document: dict[str, Any] = self._serializer.dump_dict(entry=managed_entry)
362+
document: dict[str, Any] = self._serializer.dump_dict(entry=managed_entry, key=key, collection=collection)
363363

364364
try:
365365
_ = await self._client.index( # type: ignore[reportUnknownVariableType]
@@ -395,7 +395,7 @@ async def _put_managed_entries(
395395

396396
index_action: dict[str, Any] = new_bulk_action(action="index", index=index_name, document_id=document_id)
397397

398-
document: dict[str, Any] = self._serializer.dump_dict(entry=managed_entry)
398+
document: dict[str, Any] = self._serializer.dump_dict(entry=managed_entry, key=key, collection=collection)
399399

400400
operations.extend([index_action, document])
401401

key-value/key-value-aio/tests/stores/opensearch/test_opensearch.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
import pytest
77
from dirty_equals import IsFloat, IsStr
8-
from elasticsearch import AsyncElasticsearch
98
from inline_snapshot import snapshot
109
from key_value.shared.stores.wait import async_wait_for_true
1110
from key_value.shared.utils.managed_entry import ManagedEntry
@@ -75,6 +74,7 @@ def test_managed_entry_document_conversion():
7574

7675
assert document == snapshot(
7776
{
77+
"version": 1,
7878
"value": {"flat": {"test": "test"}},
7979
"created_at": "2025-01-01T00:00:00+00:00",
8080
"expires_at": "2025-01-01T00:00:10+00:00",
@@ -189,7 +189,7 @@ async def test_put_put_two_indices(self, store: OpenSearchStore, opensearch_clie
189189
index_names: list[str] = list(indices.keys())
190190
assert index_names == snapshot(["opensearch-kv-store-e2e-test-test_collection", "opensearch-kv-store-e2e-test-test_collection_2"])
191191

192-
async def test_value_stored_as_f_object(self, store: OpenSearchStore, opensearch_client: AsyncElasticsearch):
192+
async def test_value_stored_as_f_object(self, store: OpenSearchStore, opensearch_client: AsyncOpenSearch):
193193
"""Verify values are stored as f objects, not JSON strings"""
194194
await store.put(collection="test", key="test_key", value={"name": "Alice", "age": 30})
195195

@@ -199,6 +199,9 @@ async def test_value_stored_as_f_object(self, store: OpenSearchStore, opensearch
199199
response = await opensearch_client.get(index=index_name, id=doc_id)
200200
assert response["_source"] == snapshot(
201201
{
202+
"version": 1,
203+
"key": "test_key",
204+
"collection": "test",
202205
"value": {"flat": {"name": "Alice", "age": 30}},
203206
"created_at": IsStr(min_length=20, max_length=40),
204207
}
@@ -209,6 +212,9 @@ async def test_value_stored_as_f_object(self, store: OpenSearchStore, opensearch
209212
response = await opensearch_client.get(index=index_name, id=doc_id)
210213
assert response["_source"] == snapshot(
211214
{
215+
"version": 1,
216+
"key": "test_key",
217+
"collection": "test",
212218
"value": {"flat": {"name": "Bob", "age": 25}},
213219
"created_at": IsStr(min_length=20, max_length=40),
214220
"expires_at": IsStr(min_length=20, max_length=40),

key-value/key-value-sync/src/key_value/sync/code_gen/stores/opensearch/store.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
get_source_from_body,
3838
)
3939
except ImportError as e:
40-
msg = "OpenSearchStore requires opensearch-py[async]>=2.0.0. Install with: pip install 'py-key-value-aio[opensearch]'"
40+
msg = "OpenSearchStore requires opensearch-py>=2.0.0. Install with: pip install 'py-key-value-sync[opensearch]'"
4141
raise ImportError(msg) from e
4242

4343
logger = logging.getLogger(__name__)
@@ -318,7 +318,7 @@ def _put_managed_entry(self, *, key: str, collection: str, managed_entry: Manage
318318
index_name: str = self._get_index_name(collection=collection)
319319
document_id: str = self._get_document_id(key=key)
320320

321-
document: dict[str, Any] = self._serializer.dump_dict(entry=managed_entry)
321+
document: dict[str, Any] = self._serializer.dump_dict(entry=managed_entry, key=key, collection=collection)
322322

323323
try: # type: ignore[reportUnknownVariableType]
324324
_ = self._client.index(index=index_name, id=document_id, body=document, params={"refresh": "true"})
@@ -349,7 +349,7 @@ def _put_managed_entries(
349349

350350
index_action: dict[str, Any] = new_bulk_action(action="index", index=index_name, document_id=document_id)
351351

352-
document: dict[str, Any] = self._serializer.dump_dict(entry=managed_entry)
352+
document: dict[str, Any] = self._serializer.dump_dict(entry=managed_entry, key=key, collection=collection)
353353

354354
operations.extend([index_action, document])
355355

key-value/key-value-sync/tests/code_gen/stores/opensearch/test_opensearch.py

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
import pytest
1010
from dirty_equals import IsFloat, IsStr
11-
from elasticsearch import Elasticsearch
1211
from inline_snapshot import snapshot
1312
from key_value.shared.stores.wait import wait_for_true
1413
from key_value.shared.utils.managed_entry import ManagedEntry
@@ -74,7 +73,12 @@ def test_managed_entry_document_conversion():
7473
document = adapter.dump_dict(entry=managed_entry)
7574

7675
assert document == snapshot(
77-
{"value": {"flat": {"test": "test"}}, "created_at": "2025-01-01T00:00:00+00:00", "expires_at": "2025-01-01T00:00:10+00:00"}
76+
{
77+
"version": 1,
78+
"value": {"flat": {"test": "test"}},
79+
"created_at": "2025-01-01T00:00:00+00:00",
80+
"expires_at": "2025-01-01T00:00:10+00:00",
81+
}
7882
)
7983

8084
round_trip_managed_entry = adapter.load_dict(data=document)
@@ -179,7 +183,7 @@ def test_put_put_two_indices(self, store: OpenSearchStore, opensearch_client: Op
179183
index_names: list[str] = list(indices.keys())
180184
assert index_names == snapshot(["opensearch-kv-store-e2e-test-test_collection", "opensearch-kv-store-e2e-test-test_collection_2"])
181185

182-
def test_value_stored_as_f_object(self, store: OpenSearchStore, opensearch_client: Elasticsearch):
186+
def test_value_stored_as_f_object(self, store: OpenSearchStore, opensearch_client: OpenSearch):
183187
"""Verify values are stored as f objects, not JSON strings"""
184188
store.put(collection="test", key="test_key", value={"name": "Alice", "age": 30})
185189

@@ -188,14 +192,23 @@ def test_value_stored_as_f_object(self, store: OpenSearchStore, opensearch_clien
188192

189193
response = opensearch_client.get(index=index_name, id=doc_id)
190194
assert response["_source"] == snapshot(
191-
{"value": {"flat": {"name": "Alice", "age": 30}}, "created_at": IsStr(min_length=20, max_length=40)}
195+
{
196+
"version": 1,
197+
"key": "test_key",
198+
"collection": "test",
199+
"value": {"flat": {"name": "Alice", "age": 30}},
200+
"created_at": IsStr(min_length=20, max_length=40),
201+
}
192202
)
193203

194204
# Test with TTL
195205
store.put(collection="test", key="test_key", value={"name": "Bob", "age": 25}, ttl=10)
196206
response = opensearch_client.get(index=index_name, id=doc_id)
197207
assert response["_source"] == snapshot(
198208
{
209+
"version": 1,
210+
"key": "test_key",
211+
"collection": "test",
199212
"value": {"flat": {"name": "Bob", "age": 25}},
200213
"created_at": IsStr(min_length=20, max_length=40),
201214
"expires_at": IsStr(min_length=20, max_length=40),

0 commit comments

Comments
 (0)