Skip to content

api: add Client #87

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

Merged
merged 8 commits into from
Nov 23, 2022
Merged

api: add Client #87

merged 8 commits into from
Nov 23, 2022

Conversation

hexfusion
Copy link
Contributor

@hexfusion hexfusion commented Nov 23, 2022

This PR moves the core logic out of spaces-cli and adds it to the api module. This allows for custom implementations of the client and simplifies testing. The new api client includes methods which make it easier to debug RPC calls by having direct access to the request object.

In addition this PR will add integration tests which perform round trip requests to the API to reduce time required to debug e2e tests.

This work is needed for #85

Signed-off-by: Sam Batschelet [email protected]

@hexfusion hexfusion self-assigned this Nov 23, 2022
@hexfusion hexfusion added this to the [email protected] milestone Nov 23, 2022
Signed-off-by: Sam Batschelet <[email protected]>
Signed-off-by: Sam Batschelet <[email protected]>
Signed-off-by: Sam Batschelet <[email protected]>
Signed-off-by: Sam Batschelet <[email protected]>
Signed-off-by: Sam Batschelet <[email protected]>
Signed-off-by: Sam Batschelet <[email protected]>
@hexfusion hexfusion marked this pull request as ready for review November 23, 2022 16:36
assert!(resp.is_ok());
let resp = resp.unwrap();
let body = std::str::from_utf8(&resp.body()).unwrap();
log::info!("ping response {}", body);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thee are very verbose eventually we can add more helpers.

@@ -27,6 +27,7 @@ eip-712 = "0.1.0"
env_logger = "0.9.3"
hex = "0.4.3"
http = "0.2.8"
hyper = "0.14.23"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can create a new feature client and make stuff like hyper optional.

Copy link
Contributor

@exdx exdx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! Just some feedback.

}

#[tokio::main]
async fn main() -> Result<(), Box<dyn error::Error>> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option is to use the anyhow::Result<> type as the response type. I especially like it for CLI tools. We don't have to end up with a type erased box in the signature.

For an example, see https://github.com/exdx/dcp/blob/main/src/main.rs

We could tackle this as a follow-up though.

Signed-off-by: Sam Batschelet <[email protected]>
Signed-off-by: Sam Batschelet <[email protected]>
@hexfusion hexfusion merged commit 59ddab1 into main Nov 23, 2022
@hexfusion hexfusion deleted the hyper-cli branch November 23, 2022 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants