Skip to content

Commit 9b5c68f

Browse files
committed
refactor: add Store.to_dict() and use it in connection + tests
Address review feedback: - Add to_dict() method on Store for WebSocket request serialization - Use store.to_dict() in Connection.__execute_sql instead of inline dict - Replace duplicated _serialize_store() helper in tests with to_dict() - Remove test_options_dict_is_mutable to avoid enshrining mutability
1 parent a7be067 commit 9b5c68f

File tree

3 files changed

+24
-31
lines changed

3 files changed

+24
-31
lines changed

tests/test_store_options.py

Lines changed: 6 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,6 @@ def test_options_defensively_copied(self):
3535
original["delimiter"] = "|"
3636
assert "delimiter" not in store.options
3737

38-
def test_options_dict_is_mutable(self):
39-
"""Store is not frozen, so options dict can be mutated after construction."""
40-
store = Store(format=StorageFormat.CSV, options={"header": "false"})
41-
store.options["delimiter"] = "|"
42-
assert store.options == {"header": "false", "delimiter": "|"}
43-
4438

4539
class TestStoreForDownloadWithOptions:
4640
"""Tests for Store.for_download() with options parameter."""
@@ -69,22 +63,11 @@ def test_for_download_empty_options_normalized(self):
6963

7064

7165
class TestStoreSerializationWithOptions:
72-
"""Tests for store dict serialization matching connection.py's format."""
73-
74-
def _serialize_store(self, store: Store) -> dict:
75-
"""Replicate the serialization logic from Connection.__execute_sql."""
76-
store_dict = {
77-
"format": store.format.value,
78-
"single": str(store.single).lower(),
79-
"generate_presigned_url": str(store.generate_presigned_url).lower(),
80-
}
81-
if store.options:
82-
store_dict["options"] = store.options
83-
return store_dict
66+
"""Tests for Store.to_dict() serialization."""
8467

8568
def test_serialize_without_options(self):
8669
store = Store.for_download(format=StorageFormat.GEOJSON)
87-
d = self._serialize_store(store)
70+
d = store.to_dict()
8871
assert d == {
8972
"format": "geojson",
9073
"single": "true",
@@ -97,7 +80,7 @@ def test_serialize_with_options(self):
9780
format=StorageFormat.CSV,
9881
options={"header": "false", "delimiter": "|"},
9982
)
100-
d = self._serialize_store(store)
83+
d = store.to_dict()
10184
assert d == {
10285
"format": "csv",
10386
"single": "true",
@@ -107,15 +90,15 @@ def test_serialize_with_options(self):
10790

10891
def test_serialize_empty_options_omitted(self):
10992
store = Store(format=StorageFormat.PARQUET, options={})
110-
d = self._serialize_store(store)
93+
d = store.to_dict()
11194
assert "options" not in d
11295

11396
def test_json_roundtrip_with_options(self):
11497
store = Store.for_download(
11598
format=StorageFormat.GEOJSON,
11699
options={"ignoreNullFields": "false"},
117100
)
118-
d = self._serialize_store(store)
101+
d = store.to_dict()
119102
payload = json.dumps(d)
120103
parsed = json.loads(payload)
121104
assert parsed["options"] == {"ignoreNullFields": "false"}
@@ -131,7 +114,7 @@ def test_full_request_shape(self):
131114
"execution_id": "test-id",
132115
"statement": "SELECT 1",
133116
}
134-
store_dict = self._serialize_store(store)
117+
store_dict = store.to_dict()
135118
request["store"] = store_dict
136119

137120
assert request == {

wherobots/db/connection.py

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -279,14 +279,7 @@ def __execute_sql(
279279
request["enable_progress_events"] = True
280280

281281
if store:
282-
store_dict: dict[str, Any] = {
283-
"format": store.format.value,
284-
"single": str(store.single).lower(),
285-
"generate_presigned_url": str(store.generate_presigned_url).lower(),
286-
}
287-
if store.options:
288-
store_dict["options"] = store.options
289-
request["store"] = store_dict
282+
request["store"] = store.to_dict()
290283

291284
self.__queries[execution_id] = Query(
292285
sql=sql,

wherobots/db/models.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from dataclasses import dataclass
2+
from typing import Any, Dict
23

34
import pandas
45

@@ -76,6 +77,22 @@ def for_download(
7677
options=options,
7778
)
7879

80+
def to_dict(self) -> Dict[str, Any]:
81+
"""Serialize this Store to a dict for the WebSocket request.
82+
83+
Returns a dict suitable for inclusion as the ``"store"`` field in an
84+
``execute_sql`` request. The ``options`` key is omitted when there
85+
are no user-supplied options (backward compatible).
86+
"""
87+
d: Dict[str, Any] = {
88+
"format": self.format.value,
89+
"single": str(self.single).lower(),
90+
"generate_presigned_url": str(self.generate_presigned_url).lower(),
91+
}
92+
if self.options:
93+
d["options"] = self.options
94+
return d
95+
7996

8097
@dataclass
8198
class ExecutionResult:

0 commit comments

Comments
 (0)