-
Notifications
You must be signed in to change notification settings - Fork 13
Description
As reported by @alanshaw
@fforbeck this will always return an empty array as there's nothing awaiting on the individual fetches
freeway/src/middleware/withDelegationsStorage.js
Lines 42 to 51 in 0f853ec
const delegations = [] const result = await env.CONTENT_SERVE_DELEGATIONS_STORE.list({ prefix: space }) result.keys.forEach(async (key) => { const delegation = await env.CONTENT_SERVE_DELEGATIONS_STORE.get(key.name, 'arrayBuffer') if (delegation) { const d = await Delegation.extract(new Uint8Array(delegation)) if (d.ok) delegations.push(d.ok) } }) return ok(delegations)
There's likely not enough test coverage here.
Additionally, list operations have the same cost as "writes" which will become expensive as eventually the majority of incoming requests will require it. It also has a performance penalty - an additional round trip to the KV for every delegation in the list...
I think this needs more thought and a refactor.
🙏 for the future, another review should have been requested as there was a lot of changes added after I originally approved #133
I am going to disable FF_DELEGATIONS_STORAGE_ENABLED in staging as this feature does not work.
Approach to fix the issue:
- Fix the immediate issue and add more tests
- Re-think the strategy to find these delegations and write the proposal in an RFC
Sub-issues
Metadata
Metadata
Assignees
Labels
Type
Projects
Status