Fix wrong test assertion #46
Conversation
| }); | ||
|
|
||
| // The list should now contain one execution, the id from manual trigger should matched | ||
| const execution = await client.getExecution(workflowId, result.executionId); |
There was a problem hiding this comment.
@chrisli30 This is a new API I added.
When you know the task id, and the executionid, then you can fetch a single execution. Use case:
When you call triggerWorkflow in blocking mode, you will get back the execution_id. At this time there are 2 way to get the full log of that exectuion id.
- call listExecutions and pick the first element (desc order)
- call getExecution as shown here
There was a problem hiding this comment.
Make sense, but I’ve got a question.
The isBlocking is to wait for the exeuction to complete, right? My understanding is that:
When isBlocking: true, the triggerWorkflow waits for execution to complete, at this point, it can get the result of getExecution.
Otherwise, when isBlocking: false, the triggerWorkflow runs in async mode, that it will return a tracking id immediately, which can be used later to getExecution. Also, we might need to define a response for pending, in case the getExecution is called too quickly.
Let me know if the two cases make sense. From the Studio, we would always run it in isBlocking mode, and wait for the executiond details.
There was a problem hiding this comment.
When isBlocking: true, the triggerWorkflow waits for execution to complete, at this point, it can get the result of getExecution.
Correct. The execution is done. That's why you had an "execution_id". When it's run in async mode, you don't had the executio_id. You just had result: true to mean the trigger is completed. But when it's run, you don't know yet.
Otherwise, when isBlocking: false, the triggerWorkflow runs in async mode, that it will return a tracking id immediately, which can be used later to getExecution. Also, we might need to define a response for pending, in case the getExecution is called too quickly.
No, do you remember other issue where you ask "What is the job id"? In isBlocking: false you don't get back an execution id. You just get back a job_id but we removed it.
When you had an execution id, the execution is finished.
You can safely call getExecution immediately to get the whole log. Ideally we can return the whole execution log as well but I want to keep it the triggerWorkflow response small (so it's very similar in both blocking vs non blocking call).
Let me know if the two cases make sense. From the Studio, we would always run it in isBlocking mode, and wait for the executiond details.
it make sense but we're working as your expected. I think it is just your confusuion about execution_id. Want to make it clear that if you had an execution_id, the trigger is executed, the execution is finished (it may error out such as rest endpoint is down, graphql return wrong datashape but it's execution is completed. If it's async, you don't get an id.
|
|
||
| # TODO: remove this before merge | ||
| # Pin to this branch to have all the fix | ||
| AVS_BUILD_VERSION: passing-result-to-subsequent-steps |
There was a problem hiding this comment.
@chrisli30 Once all the AVS PR is merged we can remove this so it's pointed to lastest. right now I pinned it to this branch https://github.com/AvaProtocol/ava-sdk-js/pull/45/files#diff-bccd87d19079cf7b35274281848f7e5a32a070e5101bff9c5b67b58fb82fc16dR15 and you can see all the tests are passed.
| }); | ||
|
|
||
| // The list should now contain one execution, the id from manual trigger should matched | ||
| const execution = await client.getExecution(workflowId, result.executionId); |
There was a problem hiding this comment.
Make sense, but I’ve got a question.
The isBlocking is to wait for the exeuction to complete, right? My understanding is that:
When isBlocking: true, the triggerWorkflow waits for execution to complete, at this point, it can get the result of getExecution.
Otherwise, when isBlocking: false, the triggerWorkflow runs in async mode, that it will return a tracking id immediately, which can be used later to getExecution. Also, we might need to define a response for pending, in case the getExecution is called too quickly.
Let me know if the two cases make sense. From the Studio, we would always run it in isBlocking mode, and wait for the executiond details.
|
@v9n I squash merged the previous PR but it creates conflicts. Sorry ... could you go over and resolve them? In future, should the one on top be |
No problem let me fix it.
Yes, you would merge in the reverse order from top most, then squash the last one, not other way around. A -> B -> C -> D Then you would merge D -> C -> into B, then finally inturn merge B into A(at this time you're safe to squash). When squashing B into A, that's a new commit. And all the old commit is lost. But these commit are in C, that's why it will replay and cause the conflict. |
Fix
AvaProtocol/EigenLayer-AVS#71
AvaProtocol/EigenLayer-AVS#73
AvaProtocol/EigenLayer-AVS#74
AvaProtocol/EigenLayer-AVS#75
AvaProtocol/EigenLayer-AVS#76
AvaProtocol/EigenLayer-AVS#77
AvaProtocol/EigenLayer-AVS#78
AvaProtocol/EigenLayer-AVS#79
AvaProtocol/EigenLayer-AVS#80
AvaProtocol/EigenLayer-AVS#81
AvaProtocol/EigenLayer-AVS#82