Skip to content
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

Canister query and call commands #39

Merged
merged 5 commits into from
Sep 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions dfx/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions dfx/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ console = "0.7.7"
flate2 = "1.0.11"
futures = "0.1.28"
indicatif = "0.12.0"
rand = "0.7.2"
reqwest = "0.9.20"
serde = "1.0"
serde_bytes = "0.11.2"
Expand Down
61 changes: 0 additions & 61 deletions dfx/src/commands/call.rs

This file was deleted.

53 changes: 53 additions & 0 deletions dfx/src/commands/canister/call.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
use crate::lib::api_client::{call, Blob};
use crate::lib::env::ClientEnv;
use crate::lib::error::DfxResult;
use crate::lib::CanisterId;
use crate::util::clap::validators;
use clap::{App, Arg, ArgMatches, SubCommand};
use tokio::runtime::Runtime;

pub fn construct() -> App<'static, 'static> {
SubCommand::with_name("call")
.about("Call a canister.")
.arg(
Arg::with_name("canister")
.takes_value(true)
.help("The canister ID (a number) to call.")
.required(true)
.validator(validators::is_canister_id),
)
.arg(
Arg::with_name("method_name")
.help("The method name file to use.")
.required(true),
)
.arg(
Arg::with_name("arguments")
.help("Arguments to pass to the method.")
.takes_value(true)
.multiple(true),
)
}

pub fn exec<T>(env: &T, args: &ArgMatches<'_>) -> DfxResult
where
T: ClientEnv,
{
// Read the config.
let canister_id = args.value_of("canister").unwrap().parse::<CanisterId>()?;
let method_name = args.value_of("method_name").unwrap();
let arguments: Option<Vec<&str>> = args.values_of("arguments").map(|args| args.collect());

let client = env.get_client();
let install = call(
client,
canister_id,
method_name.to_owned(),
arguments.map(|args| Blob(Vec::from(args[0]))),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a possibility that args could be empty and args[0] be out of bounds index access?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, but this isn't represented in typing. values_of() will return None if there is no "arguments", so it needs at least 1 to have some value.

);

let mut runtime = Runtime::new().expect("Unable to create a runtime");
runtime.block_on(install)?;

Ok(())
}
12 changes: 4 additions & 8 deletions dfx/src/commands/canister/install.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
use crate::lib::api_client::{install_code, Blob};
use crate::lib::env::{ClientEnv, ProjectConfigEnv};
use crate::lib::error::DfxResult;
use crate::lib::CanisterId;
use crate::util::clap::validators;
use clap::{App, Arg, ArgMatches, SubCommand};
use std::path::PathBuf;
use tokio::runtime::Runtime;

fn is_number(v: String) -> Result<(), String> {
v.parse::<u64>()
.map_err(|_| String::from("The value must be a number."))
.map(|_| ())
}

pub fn construct() -> App<'static, 'static> {
SubCommand::with_name("install")
.about("Install a canister.")
Expand All @@ -19,7 +15,7 @@ pub fn construct() -> App<'static, 'static> {
.takes_value(true)
.help("The canister ID (a number).")
.required(true)
.validator(is_number),
.validator(validators::is_canister_id),
)
.arg(
Arg::with_name("wasm")
Expand All @@ -36,7 +32,7 @@ where
let config = env.get_config().unwrap();
let project_root = config.get_path().parent().unwrap();

let canister_id = args.value_of("canister").unwrap().parse::<u64>()?;
let canister_id = args.value_of("canister").unwrap().parse::<CanisterId>()?;
let wasm_path = args.value_of("wasm").unwrap();
let wasm_path = PathBuf::from(project_root).join(wasm_path);

Expand Down
8 changes: 7 additions & 1 deletion dfx/src/commands/canister/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,19 @@ use crate::lib::env::{ClientEnv, ProjectConfigEnv};
use crate::lib::error::{DfxError, DfxResult};
use clap::{App, ArgMatches, SubCommand};

mod call;
mod install;
mod query;

fn builtins<T>() -> Vec<CliCommand<T>>
where
T: ClientEnv + ProjectConfigEnv,
{
vec![CliCommand::new(install::construct(), install::exec)]
vec![
CliCommand::new(call::construct(), call::exec),
CliCommand::new(install::construct(), install::exec),
CliCommand::new(query::construct(), query::exec),
]
}

pub fn construct<T>() -> App<'static, 'static>
Expand Down
69 changes: 69 additions & 0 deletions dfx/src/commands/canister/query.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
use crate::lib::api_client::{query, Blob, QueryResponseReply, ReadResponse};
use crate::lib::env::ClientEnv;
use crate::lib::error::{DfxError, DfxResult};
use crate::lib::CanisterId;
use crate::util::clap::validators;
use clap::{App, Arg, ArgMatches, SubCommand};
use tokio::runtime::Runtime;

pub fn construct() -> App<'static, 'static> {
SubCommand::with_name("query")
.about("Query a canister.")
.arg(
Arg::with_name("canister")
.takes_value(true)
.help("The canister ID (a number) to query.")
.required(true)
.validator(validators::is_canister_id),
)
.arg(
Arg::with_name("method_name")
.help("The name of the method to query.")
.required(true),
)
.arg(
Arg::with_name("arguments")
.help("Arguments to pass to the method.")
.takes_value(true)
.multiple(true),
)
}

pub fn exec<T>(env: &T, args: &ArgMatches<'_>) -> DfxResult
where
T: ClientEnv,
{
// Read the config.
let canister_id = args.value_of("canister").unwrap().parse::<CanisterId>()?;
let method_name = args.value_of("method_name").unwrap();
let arguments: Option<Vec<&str>> = args.values_of("arguments").map(|args| args.collect());

let client = env.get_client();
let install = query(
client,
canister_id,
method_name.to_owned(),
arguments.map(|args| Blob(Vec::from(args[0]))),
);

let mut runtime = Runtime::new().expect("Unable to create a runtime");
match runtime.block_on(install) {
Ok(ReadResponse::Pending) => {
println!("Pending");
Ok(())
}
Ok(ReadResponse::Replied {
reply: QueryResponseReply { arg: Blob(blob) },
}) => {
println!("{}", String::from_utf8_lossy(&blob));
Ok(())
}
Ok(ReadResponse::Rejected {
reject_code,
reject_message,
}) => Err(DfxError::ClientError(reject_code, reject_message)),
// TODO: remove this when moving to ic_http_api.
Ok(ReadResponse::Unknown) => Err(DfxError::Unknown("Unknown response".to_owned())),
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is what we want to do here. The spec says:

Warning: Immediately after submitting a request, this may fail (e.g. return with unknown) even though the system is still working on accepting the request as pending.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was from the call method. I only moved the same logic.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it was wrong before as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually Query cannot return UNKNOWN, so I don't know why that's there. That would be a panic!("this should not happen"). I'll leave it here with a note that we should remove this response from query() when moving to the ic_http_api

Copy link
Contributor

Choose a reason for hiding this comment

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

In another PR I suggested having different response status types because some have pending and unknown and others don't. I think that's the way to go.

Err(x) => Err(x),
}
}
2 changes: 0 additions & 2 deletions dfx/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use crate::lib::error::DfxResult;
use clap::ArgMatches;

mod build;
mod call;
mod canister;
mod new;
mod start;
Expand Down Expand Up @@ -41,7 +40,6 @@ where
{
vec![
CliCommand::new(build::construct(), build::exec),
CliCommand::new(call::construct(), call::exec),
CliCommand::new(canister::construct::<T>(), canister::exec),
CliCommand::new(new::construct(), new::exec),
CliCommand::new(start::construct(), start::exec),
Expand Down
Loading