Optimize exec async #16
Conversation
rai-sdk/src/main/java/com/relationalai/TransactionAsyncResult.java
Outdated
Show resolved
Hide resolved
| @@ -4,15 +4,32 @@ | |||
|
|
|||
| public class TransactionAsyncResult extends Entity { | |||
|
|
|||
| public Boolean gotCompletedResult; | |||
There was a problem hiding this comment.
let's call this gotCompleteResult, not gotCompletedResult, inline with the Go SDK: https://github.com/RelationalAI/rai-sdk-go/blob/bbc1c3bba86b43865e983d992d48161e5652530f/rai/models.go#L302-L303
It doesn't mean that the transaction was completed, it means that it was either completed or aborted, and we got the results.
There was a problem hiding this comment.
I guess there are a few cases we're covering with this type:
ExecuteAsync=> fast path; contains results. state isabortedorcompletedExecuteAsync=> slow path; doesn't contain results. state isrunningExecute=> regardless of path, contains results. eitherabortedorcompleted
I guess gotCompleteResult differentiates (1) from (2)? But why do we need it? Can't we use the state to differentiate?
What am I missing @NHDaly
There was a problem hiding this comment.
We discussed how in the original spec (before you joined the project, i think, @vilterp), we intended to have a fourth state:
4. ExecuteAsync => fast path; does not contain results (because they are too big to send over the wire, and you presumably want to page them). state is aborted or completed
So i think this isn't strictly needed right now; it's a future-proofing that you and I did while we were looking at the code.
There was a problem hiding this comment.
K thanks. I forgot that it was future proofing and not a case we need now.
…java Co-authored-by: Pete Vilter <[email protected]>
|
PR merge conflicts. let us know when you're ready for us to look again! |
|
@NHDaly PR looks good again 😄 |
Return fast-path response if exists