-
Notifications
You must be signed in to change notification settings - Fork 3
feat(graphql): Allow waiting for finalization in client #349
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Doesn't compile, can't see it on GH because of the conflict |
| #[error("Missing digest for object {0}")] | ||
| MissingDigest(ObjectId), | ||
| #[error("Missing transaction for digest {0}")] | ||
| MissingTransaction(Digest), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess that ship has sailed but I always feel a bit weird this kind of stuff being an error instead of an option
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well it is an error because it should not be missing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree, it could be missing and that's fine, it's a state, not an error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm willing to approve anyway, but I don't think we should do that
|
I'll do a final review before this is merged please 🙏🏻 |
Merging this first thing tomorrow morning, one last look 👀 |
| ); | ||
|
|
||
| let res = builder.execute(&keypair.into(), true).await?; | ||
| let effects = builder.execute(&keypair.into(), None).await?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the only design flaw IMHO of Rust is that it doesn't allow for named arguments. I hate reading function calls, that have true or None in it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah :c
* feat(graphql): Allow waiting for finalization in client * bindings * go home * add doc * Improve wait fn and execute * apply to other execute fn * clippy * review * Update crates/iota-sdk-graphql-client/src/lib.rs Co-authored-by: /alex/ <[email protected]> * more bindings updates * update gas station example * fix responses and wait for checkpoint inclusion * Don't return transaction effects * fix balance query and improvements * Update schema and add `is_tx_indexed_on_node` * add WaitForTx * rename variants * typo * Update iota binary * fix tests * fix more tests * fix publish example * review --------- Co-authored-by: /alex/ <[email protected]>
Description
Adds the
wait_for_tx_finalizationfn and thewait_for_finalizationboolean flag to theexecute_txfn to allow for waiting in the client. I also changed the way we wait, so it is now a duration timeout rather than a number of retries.Closes #316