Skip to content

Commit 8934996

Browse files
rustyconoverclaude
andcommitted
function_storage: shard_key routing for CF DO + tx_cached_value fixture
Threads a shard_key through every FunctionStorage method so the CF DO backend can route storage ops to a stable Durable Object instance per attach (or per authenticated user, or anonymous fallback). BoundStorage and TransactionBoundStorage derive the key from request.attach_id / auth.principal via _derive_shard_key and pass it to every backend call. - function_storage.py: add _derive_shard_key (attach_id → "att-<hex>", authenticated principal → "prn-<sha256>", anonymous → "loc-anon"); shard_key=str kwarg on every Protocol method; BoundStorage and TransactionBoundStorage accept request= / attach_id= / auth= and thread shard_key through. SQLite backend ignores shard_key (no sharding). - function_storage_cf_do.py / function_storage_azure_sql.py: shard_key threaded through every method (CF DO uses it as the DO routing key; Azure ignores it). - protocol.py, scalar_function.py, table_function.py, table_in_out_function.py: pass request= / auth= to BoundStorage and TransactionBoundStorage at every construction site. - tests/test_function_storage*.py: cover the new kwarg and the derive precedence. - vgi/_test_fixtures/table/transaction_storage.py: new tx_cached_value fixture exercising bind→init→process opaque-data round-trip via BindResponse.opaque_data + transaction_storage caching. on_init declares max_workers=1 because the function emits exactly one row with no work-queue coordination. - vgi/_test_fixtures/table/__init__.py + worker.py: register the new fixture. - profiling_example.py: minor adjustment. - worker.py: minor adjustment to init dispatch. - docs/shared-storage.md: point at the relocated CF DO repo. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent b343754 commit 8934996

18 files changed

Lines changed: 613 additions & 312 deletions

docs/shared-storage.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,12 @@ all operations are inherently atomic without locking.
110110

111111
### Setup
112112

113-
1. Deploy the Cloudflare Worker (from `cloudflare/vgi-storage/`):
113+
1. Deploy the Cloudflare Worker. The Worker source lives in a separate
114+
repository: [`vgi-cloudflare-durable-object-storage`](https://github.com/query-farm/vgi-cloudflare-durable-object-storage).
114115

115116
```bash
116-
cd cloudflare/vgi-storage
117+
git clone https://github.com/query-farm/vgi-cloudflare-durable-object-storage
118+
cd vgi-cloudflare-durable-object-storage
117119
npm install
118120
npx wrangler deploy
119121
```

tests/test_function_storage.py

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
import pytest
66

7-
from vgi.function_storage import FunctionStorageSqlite, UnknownInvocationError
7+
from vgi.function_storage import FunctionStorageSqlite
88

99

1010
class TestFunctionStorageSqlite:
@@ -106,11 +106,13 @@ def test_queue_pop_empty_queue(self, storage: FunctionStorageSqlite) -> None:
106106
result = storage.queue_pop(invocation_id)
107107
assert result is None
108108

109-
def test_queue_pop_unknown_invocation_raises(self, storage: FunctionStorageSqlite) -> None:
110-
"""Test popping from unknown invocation raises error."""
109+
def test_queue_pop_never_pushed_returns_none(self, storage: FunctionStorageSqlite) -> None:
110+
"""Test popping an id that was never pushed returns None.
111+
112+
No distinction from drained queue per the contract.
113+
"""
111114
unknown_id = b"never_seen_before"
112-
with pytest.raises(UnknownInvocationError):
113-
storage.queue_pop(unknown_id)
115+
assert storage.queue_pop(unknown_id) is None
114116

115117
def test_queue_clear(self, storage: FunctionStorageSqlite) -> None:
116118
"""Test clearing the work queue."""
@@ -123,26 +125,8 @@ def test_queue_clear(self, storage: FunctionStorageSqlite) -> None:
123125
cleared = storage.queue_clear(invocation_id)
124126
assert cleared == 3
125127

126-
# After clear, invocation is unregistered so pop should raise
127-
with pytest.raises(UnknownInvocationError):
128-
storage.queue_pop(invocation_id)
129-
130-
def test_queue_clear_unregisters_invocation(self, storage: FunctionStorageSqlite) -> None:
131-
"""Test that queue_clear unregisters the invocation_id."""
132-
invocation_id = b"inv123"
133-
134-
# Register by pushing
135-
storage.queue_push(invocation_id, [b"item1"])
136-
137-
# Pop should work (known invocation)
138-
storage.queue_pop(invocation_id)
139-
140-
# Clear unregisters
141-
storage.queue_clear(invocation_id)
142-
143-
# Now pop should raise (unknown after clear)
144-
with pytest.raises(UnknownInvocationError):
145-
storage.queue_pop(invocation_id)
128+
# After clear, pop returns None (queue is empty/unknown)
129+
assert storage.queue_pop(invocation_id) is None
146130

147131
def test_queue_push_empty_still_registers(self, storage: FunctionStorageSqlite) -> None:
148132
"""Test that pushing empty list still registers invocation_id."""

tests/test_function_storage_azure_sql.py

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

99
pymssql = pytest.importorskip("pymssql")
1010

11-
from vgi.function_storage import UnknownInvocationError # noqa: E402
1211
from vgi.function_storage_azure_sql import FunctionStorageAzureSql # noqa: E402
1312

1413

@@ -155,8 +154,8 @@ def test_queue_pop_returns_item(self, storage: FunctionStorageAzureSql, mock_cur
155154
"""Test that queue_pop returns an item when available."""
156155
execution_id = b"\x02" * 16
157156

158-
# Combined SQL returns (registered, work_item)
159-
mock_cursor.set_results([(1, b"item1")])
157+
# OUTPUT deleted.work_item returns one row with the item bytes.
158+
mock_cursor.set_results([(b"item1",)])
160159

161160
result = storage.queue_pop(execution_id)
162161
assert result == b"item1"
@@ -169,20 +168,22 @@ def test_queue_pop_empty_queue(self, storage: FunctionStorageAzureSql, mock_curs
169168
"""Test popping from registered but empty queue returns None."""
170169
execution_id = b"\x02" * 16
171170

172-
# Combined SQL returns (registered=1, work_item=NULL) for empty queue
173-
mock_cursor.set_results([(1, None)])
171+
# No row deleted → no OUTPUT row.
172+
mock_cursor.set_results([])
174173

175174
result = storage.queue_pop(execution_id)
176175
assert result is None
177176

178-
def test_queue_pop_unknown_invocation_raises(
177+
def test_queue_pop_never_pushed_returns_none(
179178
self, storage: FunctionStorageAzureSql, mock_cursor: _MockCursor
180179
) -> None:
181-
"""Test popping from unknown invocation raises error."""
182-
# Combined SQL returns (registered=0, NULL) for unregistered invocation
183-
mock_cursor.set_results([(0, None)])
184-
with pytest.raises(UnknownInvocationError):
185-
storage.queue_pop(b"\xff" * 16)
180+
"""Popping an id that was never pushed returns None.
181+
182+
No distinction from drained queue per the contract.
183+
"""
184+
# Same wire shape as empty queue — no OUTPUT row.
185+
mock_cursor.set_results([])
186+
assert storage.queue_pop(b"\xff" * 16) is None
186187

187188
def test_queue_push_empty_list(self, storage: FunctionStorageAzureSql, mock_cursor: _MockCursor) -> None:
188189
"""Test pushing empty list returns 0."""

tests/test_function_storage_cf_do.py

Lines changed: 12 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import httpx
1212
import pytest
1313

14-
from vgi.function_storage import UnknownInvocationError
1514
from vgi.function_storage_cf_do import FunctionStorageCfDo
1615

1716

@@ -162,14 +161,6 @@ def test_queue_pop_empty_queue(self, storage: FunctionStorageCfDo, mock_transpor
162161
result = storage.queue_pop(b"\x02" * 16)
163162
assert result is None
164163

165-
def test_queue_pop_unknown_invocation_raises(
166-
self, storage: FunctionStorageCfDo, mock_transport: _MockTransport
167-
) -> None:
168-
"""Test popping from unknown invocation raises error."""
169-
mock_transport.queue_response(404, {"error": "unknown_invocation"})
170-
with pytest.raises(UnknownInvocationError):
171-
storage.queue_pop(b"\xff" * 16)
172-
173164
def test_queue_push_empty_list(self, storage: FunctionStorageCfDo, mock_transport: _MockTransport) -> None:
174165
"""Test pushing empty list returns 0."""
175166
mock_transport.queue_response(200, {"count": 0})
@@ -192,17 +183,6 @@ def test_queue_clear(self, storage: FunctionStorageCfDo, mock_transport: _MockTr
192183
cleared = storage.queue_clear(b"\x02" * 16)
193184
assert cleared == 3
194185

195-
def test_queue_clear_unregisters_invocation(
196-
self, storage: FunctionStorageCfDo, mock_transport: _MockTransport
197-
) -> None:
198-
"""Test that queue_clear causes subsequent pop to raise."""
199-
mock_transport.queue_response(200, {"cleared": 0})
200-
storage.queue_clear(b"\x02" * 16)
201-
202-
mock_transport.queue_response(404, {"error": "unknown_invocation"})
203-
with pytest.raises(UnknownInvocationError):
204-
storage.queue_pop(b"\x02" * 16)
205-
206186
def test_queue_clear_empty_queue(self, storage: FunctionStorageCfDo, mock_transport: _MockTransport) -> None:
207187
"""Test clearing an empty queue returns 0."""
208188
mock_transport.queue_response(200, {"cleared": 0})
@@ -423,7 +403,7 @@ def _destructive_call_cases(
423403
return [
424404
("worker_put", "worker_put"),
425405
("worker_collect", "worker_collect"),
426-
("scan_worker_put", "scan_worker_put"),
406+
("stream_state_put", "stream_state_put"),
427407
("queue_push", "queue_push"),
428408
("queue_pop", "queue_pop"),
429409
("queue_clear", "queue_clear"),
@@ -449,8 +429,8 @@ def _invoke(
449429
storage.worker_put(eid, worker_id=1, state=b"x")
450430
elif method_name == "worker_collect":
451431
storage.worker_collect(eid)
452-
elif method_name == "scan_worker_put":
453-
storage.scan_worker_put(eid, sid, b"x")
432+
elif method_name == "stream_state_put":
433+
storage.stream_state_put(eid, sid, b"x")
454434
elif method_name == "queue_push":
455435
storage.queue_push(eid, [b"a", b"b"])
456436
elif method_name == "queue_pop":
@@ -507,7 +487,7 @@ def test_read_only_calls_do_not_send_attempt_id(
507487
mock_transport.queue_response(200, {"rows": []})
508488
storage.worker_scan(b"\x01" * 16)
509489
mock_transport.queue_response(200, {"rows": []})
510-
storage.scan_worker_scan(b"\x01" * 16)
490+
storage.stream_state_scan(b"\x01" * 16)
511491
mock_transport.queue_response(200, {"values": [None]})
512492
storage.transaction_state_get(b"\xaa" * 16, [b"k"])
513493
mock_transport.queue_response(200, {"rows": [None]})
@@ -561,20 +541,22 @@ def test_attempt_id_unique_per_logical_call(
561541

562542
# --- Scan Worker State Tests ---
563543

564-
def test_scan_worker_put_round_trip(self, storage: FunctionStorageCfDo, mock_transport: _MockTransport) -> None:
565-
"""scan_worker_put sends execution_id/stream_id/state and an attempt_id."""
544+
def test_stream_state_put_round_trip(self, storage: FunctionStorageCfDo, mock_transport: _MockTransport) -> None:
545+
"""stream_state_put sends execution_id/stream_id/state and an attempt_id."""
566546
eid = b"\x01" * 16
567547
sid = b"\xde" * 16
568548
mock_transport.queue_response(200, {})
569-
storage.scan_worker_put(eid, sid, b"payload")
549+
storage.stream_state_put(eid, sid, b"payload")
570550
body = _body(mock_transport.requests[0])
571551
assert base64.b64decode(body["execution_id"]) == eid
572552
assert base64.b64decode(body["stream_id"]) == sid
573553
assert base64.b64decode(body["state"]) == b"payload"
574554
assert self._ATTEMPT_RE.match(body["attempt_id"])
575555

576-
def test_scan_worker_scan_returns_pairs(self, storage: FunctionStorageCfDo, mock_transport: _MockTransport) -> None:
577-
"""scan_worker_scan returns (stream_id, state) tuples and carries no attempt_id."""
556+
def test_stream_state_scan_returns_pairs(
557+
self, storage: FunctionStorageCfDo, mock_transport: _MockTransport
558+
) -> None:
559+
"""stream_state_scan returns (stream_id, state) tuples and carries no attempt_id."""
578560
sid_a = b"\xde" * 16
579561
sid_b = b"\xad" * 16
580562
mock_transport.queue_response(
@@ -586,7 +568,7 @@ def test_scan_worker_scan_returns_pairs(self, storage: FunctionStorageCfDo, mock
586568
],
587569
},
588570
)
589-
rows = storage.scan_worker_scan(b"\x01" * 16)
571+
rows = storage.stream_state_scan(b"\x01" * 16)
590572
assert rows == [(sid_a, b"alpha"), (sid_b, b"beta")]
591573

592574
# --- Aggregate State Tests ---

tests/test_function_storage_cf_do_integration.py

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import httpx
2323
import pytest
2424

25-
from vgi.function_storage import UnknownInvocationError
2625
from vgi.function_storage_cf_do import FunctionStorageCfDo
2726

2827
# Skip the entire module unless explicitly enabled.
@@ -193,37 +192,38 @@ def test_queue_pop_replay_returns_same_item(storage: FunctionStorageCfDo) -> Non
193192
assert storage.queue_pop(eid) == b"b"
194193

195194

196-
def test_queue_pop_unknown_invocation(storage: FunctionStorageCfDo) -> None:
197-
"""Verify pop against a never-pushed execution_id raises UnknownInvocationError."""
198-
with pytest.raises(UnknownInvocationError):
199-
storage.queue_pop(_eid())
195+
def test_queue_pop_never_pushed_returns_none(storage: FunctionStorageCfDo) -> None:
196+
"""Pop against a never-pushed execution_id returns None.
197+
198+
No distinction from drained queue per the contract.
199+
"""
200+
assert storage.queue_pop(_eid()) is None
200201

201202

202-
def test_queue_clear_unregisters(storage: FunctionStorageCfDo) -> None:
203-
"""Clear must remove queue rows and unregister the invocation."""
203+
def test_queue_clear_then_pop_returns_none(storage: FunctionStorageCfDo) -> None:
204+
"""Clear removes queue rows; subsequent pop returns None (queue empty)."""
204205
eid = _eid()
205206
storage.queue_push(eid, [b"a", b"b"])
206207
cleared = storage.queue_clear(eid)
207208
assert cleared == 2
208209

209-
with pytest.raises(UnknownInvocationError):
210-
storage.queue_pop(eid)
210+
assert storage.queue_pop(eid) is None
211211

212212

213213
# ---------------------------------------------------------------------------
214214
# Scan worker state
215215
# ---------------------------------------------------------------------------
216216

217217

218-
def test_scan_worker_put_and_scan_round_trip(storage: FunctionStorageCfDo) -> None:
218+
def test_stream_state_put_and_scan_round_trip(storage: FunctionStorageCfDo) -> None:
219219
"""Round-trip scan worker state for two distinct stream_ids."""
220220
eid = _eid()
221221
sid_a = secrets.token_bytes(16)
222222
sid_b = secrets.token_bytes(16)
223-
storage.scan_worker_put(eid, sid_a, b"alpha")
224-
storage.scan_worker_put(eid, sid_b, b"beta")
223+
storage.stream_state_put(eid, sid_a, b"alpha")
224+
storage.stream_state_put(eid, sid_b, b"beta")
225225

226-
rows = dict(storage.scan_worker_scan(eid))
226+
rows = dict(storage.stream_state_scan(eid))
227227
assert rows == {sid_a: b"alpha", sid_b: b"beta"}
228228

229229

@@ -289,7 +289,13 @@ def test_invalid_attempt_id_format_returns_400(storage: FunctionStorageCfDo) ->
289289
import base64
290290

291291
eid = _eid()
292-
body = {"execution_id": base64.b64encode(eid).decode(), "attempt_id": "not-hex"}
292+
body = {
293+
"execution_id": base64.b64encode(eid).decode(),
294+
"attempt_id": "not-hex",
295+
# Worker rejects missing shard_key first; supply a placeholder so
296+
# the attempt_id validation downstream is what we actually exercise.
297+
"shard_key": "loc-anon",
298+
}
293299
# Drive raw httpx so we can ship an arbitrary attempt_id string.
294300
resp = storage._client.post("/queue_pop", json=body)
295301
assert resp.status_code == 400
@@ -312,10 +318,7 @@ def test_concurrent_pops_no_dupes(storage: FunctionStorageCfDo) -> None:
312318
lock = threading.Lock()
313319

314320
def pop() -> None:
315-
try:
316-
r = storage.queue_pop(eid)
317-
except UnknownInvocationError:
318-
r = None
321+
r = storage.queue_pop(eid)
319322
with lock:
320323
results.append(r)
321324

@@ -330,10 +333,14 @@ def pop() -> None:
330333
assert len(set(items)) == n_items, "no item must be popped twice"
331334

332335

333-
def test_health_check_get_returns_405(storage: FunctionStorageCfDo) -> None:
334-
"""Sanity: GET is rejected at the router."""
336+
def test_health_check_get_is_rejected(storage: FunctionStorageCfDo) -> None:
337+
"""Sanity: GET is rejected at the router.
338+
339+
Accept either 405 (when no auth token is configured) or 401 (when one
340+
is — the router validates the bearer token before method).
341+
"""
335342
resp = httpx.get(_URL or "")
336-
assert resp.status_code == 405
343+
assert resp.status_code in (401, 405)
337344

338345

339346
# ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)