feat: agent store integration #380
Conversation
| // Prior implementation used to resolve the invocation and include it's | ||
| // details the stream record. If invocation was not found it threw | ||
| // exception which would result in HTTP status 500. | ||
|
|
||
| // 🔬 Need to figure if there are consumers downstream that depend on |
There was a problem hiding this comment.
IIRC it is currently needed. There are consumers that need the information of the invocation to track metrics. For instance, when we receive a store/add receipt with 'ok', we need the invocation info to know the size value to increment the size os bytes received by the service
I would wish we just attempt these changes in follow up, or we are going to make this quite complicated
There was a problem hiding this comment.
I want to raise that except for the 2 problems still above and once we also merge here #381, everything else looks good to me and I am confident we can merge this tomorrow
| @@ -27,25 +22,34 @@ const AWS_REGION = process.env.AWS_REGION || 'us-west-2' | |||
| async function handlerFn(request) { | |||
| const { | |||
| INVOCATION_BUCKET_NAME: invocationBucketName = '', | |||
| TASK_BUCKET_NAME: taskBucketName = '', | |||
There was a problem hiding this comment.
It was never used and me and @vasco-santos realized that it would introduce extra indirection which we could avoid by using new pathing in form of
task/invocation@message.in
task/revocation@message.out
| const result = await ctx.agentStore.messages.write({ | ||
| source: request, | ||
| data: message, | ||
| index: AgentMessage.index(message) |
There was a problem hiding this comment.
Hmm...why do we still have to do this?
There was a problem hiding this comment.
Unfortunately this is yet another endpoint we have that simply receives invocations / receipts and simply writes them into a store and indexes them, so all of the stuff added in upload-api was does not cover this 🥺
I thine we could clean this up or perhaps deprecate this endpoint all together, but until we do this seemed like a best compromise I could think of
This PR fixes 3/4 integration tests failing in target PR, as well as swaps receiptsStorage in blob tests by the agent store. The remaining one is a problem from #380 (comment)
View stack outputs
|
vasco-santos
force-pushed
the
feat/agent-store-integration
branch
from
ffe8360
to
c53bc03
Compare
…1479) While working on storacha/w3infra#380 I have realized that agent-store API was poorly designed because it imposed: 1. Store to traverse agent message in order to index each invocations and receipts within it and code that dealt with traversal was trapped in util module here that was not even exported. 2. Store has to encode message into bytes in order to persist it, which is redundant given that we received message in encoded form. This PR fixes above limitations by switching store interface from receiving `AgentMessage` to `ParsedAgentMessage` which wraps `AgentMessage` along with message bytes and index freeing store from doing any kind of traversal or encoding. I also removed legacy code that was left behind by previous PR.
Integrates agent-store into the infra and removes custom invocation execution and storage that was used previously. Requires storacha/w3up#1479