-
Notifications
You must be signed in to change notification settings - Fork 0
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
kairos-cli: implement call to deposit endpoint and integration test #91
Conversation
I'm a little confused as to why you're spawning a tokio runtime to use reqwests async API in a CLI application when reqwest has a blocking API and the nature of a CLI is generally synchronous. Let's merge it, just leaving this comment here for future reference and to make everyone aware of reqwests blocking API. |
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.
Needs more work - details in comments.
@Rom3dius that's right! CLI client (replaced with Rust SDK in the future) should be synchronous, not only for simplicity, but there is no advantage of having async there. @marijanp could you use |
987177d
to
0c980ef
Compare
Minimum allowed coverage is Generated by 🐒 cobertura-action against 1da5208 |
Things done
kairos_cli::run
kairos-server-address
option, that defaults a value if not setkairos-server
, this client function introduces aKairosClientError
enum type that could be used for better error handlingOverview
#92
Motivation