-
Notifications
You must be signed in to change notification settings - Fork 98
SDK-607 feat: replace using IDs on the command line with project name #145
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
Changes from 1 commit
5c8c301
cc219e6
e808e14
0ccd739
e11d868
8f3bbe5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,35 +1,80 @@ | ||
| use crate::lib::api_client::{install_code, request_status, QueryResponseReply, ReadResponse}; | ||
| use crate::lib::api_client::{ | ||
| install_code, request_status, Client, QueryResponseReply, ReadResponse, | ||
| }; | ||
| use crate::lib::canister_info::CanisterInfo; | ||
| use crate::lib::env::{ClientEnv, ProjectConfigEnv}; | ||
| use crate::lib::error::{DfxError, DfxResult}; | ||
| use crate::lib::message::UserMessage; | ||
| use crate::util::clap::validators; | ||
| use crate::util::print_idl_blob; | ||
| use clap::{App, Arg, ArgMatches, SubCommand}; | ||
| use ic_http_agent::{Blob, CanisterId}; | ||
| use std::path::PathBuf; | ||
| use ic_http_agent::{Blob, RequestId}; | ||
| use tokio::runtime::Runtime; | ||
|
|
||
| pub fn construct() -> App<'static, 'static> { | ||
| SubCommand::with_name("install") | ||
| .about(UserMessage::InstallCanister.to_str()) | ||
| .arg( | ||
| Arg::with_name("deployment_id") | ||
| Arg::with_name("canister_name") | ||
| .takes_value(true) | ||
| .help(UserMessage::SetDeploymentId.to_str()) | ||
| .required(true) | ||
| .validator(validators::is_canister_id), | ||
| .help(UserMessage::CanisterName.to_str()) | ||
| .required(false), | ||
| ) | ||
| .arg( | ||
| Arg::with_name("async") | ||
| .help(UserMessage::AsyncResult.to_str()) | ||
| .long("async") | ||
| .takes_value(false), | ||
| ) | ||
| .arg( | ||
| Arg::with_name("wasm") | ||
| .help(UserMessage::WasmFile.to_str()) | ||
| .required(true), | ||
| ) | ||
| } | ||
|
|
||
| pub fn install_canister(client: &Client, canister_info: &CanisterInfo) -> DfxResult<RequestId> { | ||
| let canister_id = canister_info.get_canister_id().ok_or_else(|| { | ||
| DfxError::CannotFindBuildOutputForCanister(canister_info.get_name().to_owned()) | ||
| })?; | ||
|
|
||
| eprintln!( | ||
| "Installing {} with ID {}...", | ||
| canister_info.get_name(), | ||
| canister_id.to_hex(), | ||
| ); | ||
|
|
||
| let wasm_path = canister_info.get_output_wasm_path(); | ||
| let wasm = std::fs::read(wasm_path)?; | ||
|
|
||
| let install = install_code(client.clone(), canister_id, Blob::from(wasm), None); | ||
|
|
||
| let mut runtime = Runtime::new().expect("Unable to create a runtime"); | ||
| let request_id = runtime.block_on(install)?; | ||
|
|
||
| Ok(request_id) | ||
| } | ||
|
|
||
| pub fn wait_on_request_status(client: &Client, request_id: RequestId) -> DfxResult { | ||
| let request_status = request_status(client.clone(), request_id); | ||
| let mut runtime = Runtime::new().expect("Unable to create a runtime"); | ||
|
|
||
| match runtime.block_on(request_status) { | ||
| Ok(ReadResponse::Pending) => { | ||
| eprintln!("Pending"); | ||
| println!("0x{}", String::from(request_id)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we do something like this? eprint!("Request ID: ");
println!("0x{}", String::from(request_id));I'm hoping this would read
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same with other places we print the ID.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure. Note that this code hasn't been changed (copy-pasted) and we should most definitely rewrite it all when we move the client API to the http lib.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: Should we pipe things to different stdout and stderr? Why don't we use the same (stderr)?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. STDOUT is scriptable. STDERR is human readable. This is a simplification, but in essence that's what it is. Like in our scripts we use STDOUT of a command to use request-status.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the clarification! |
||
| Ok(()) | ||
| } | ||
| Ok(ReadResponse::Replied { reply }) => { | ||
| if let Some(QueryResponseReply { arg: blob }) = reply { | ||
| print_idl_blob(&blob) | ||
| .map_err(|e| DfxError::InvalidData(format!("Invalid IDL blob: {}", e)))?; | ||
| } | ||
| Ok(()) | ||
| } | ||
| Ok(ReadResponse::Rejected { | ||
| reject_code, | ||
| reject_message, | ||
| }) => Err(DfxError::ClientError(reject_code, reject_message)), | ||
| // TODO(SDK-446): remove this matcher when moving api_client to ic_http_agent. | ||
| // `install` cannot return Unknown. | ||
| Ok(ReadResponse::Unknown) => Err(DfxError::Unknown("Unknown response".to_owned())), | ||
| Err(x) => Err(x), | ||
| } | ||
| } | ||
|
|
||
| pub fn exec<T>(env: &T, args: &ArgMatches<'_>) -> DfxResult | ||
|
|
@@ -41,51 +86,31 @@ where | |
| .get_config() | ||
| .ok_or(DfxError::CommandMustBeRunInAProject)?; | ||
|
|
||
| let project_root = config.get_path().parent().unwrap(); | ||
|
|
||
| let canister_id = args | ||
| .value_of("deployment_id") | ||
| .unwrap() | ||
| .parse::<CanisterId>() | ||
| .map_err(|e| DfxError::InvalidArgument(format!("Invalid deployment ID: {}", e)))?; | ||
| let wasm_path = args.value_of("wasm").unwrap(); | ||
| let wasm_path = PathBuf::from(project_root).join(wasm_path); | ||
|
|
||
| let wasm = std::fs::read(wasm_path)?; | ||
| let client = env.get_client(); | ||
| if let Some(canister_name) = args.value_of("canister_name") { | ||
| let canister_info = CanisterInfo::load(config, canister_name)?; | ||
| let request_id = install_canister(&client, &canister_info)?; | ||
|
|
||
| let install = install_code(client, canister_id, Blob(wasm), None); | ||
|
|
||
| let mut runtime = Runtime::new().expect("Unable to create a runtime"); | ||
| let request_id = runtime.block_on(install)?; | ||
|
|
||
| if args.is_present("async") { | ||
| println!("0x{}", String::from(request_id)); | ||
| Ok(()) | ||
| if args.is_present("async") { | ||
| println!("0x{}", String::from(request_id)); | ||
| Ok(()) | ||
| } else { | ||
| wait_on_request_status(&client, request_id) | ||
| } | ||
| } else { | ||
| let request_status = request_status(env.get_client(), request_id); | ||
| let mut runtime = Runtime::new().expect("Unable to create a runtime"); | ||
| match runtime.block_on(request_status) { | ||
| Ok(ReadResponse::Pending) => { | ||
| eprintln!("Pending"); | ||
| println!("0x{}", String::from(request_id)); | ||
| Ok(()) | ||
| } | ||
| Ok(ReadResponse::Replied { reply }) => { | ||
| if let Some(QueryResponseReply { arg: blob }) = reply { | ||
| print_idl_blob(&blob) | ||
| .map_err(|e| DfxError::InvalidData(format!("Invalid IDL blob: {}", e)))?; | ||
| // Install all canisters. | ||
| if let Some(canisters) = &config.get_config().canisters { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should consider showing an error saying that a canister name or an
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
| for canister_name in canisters.keys() { | ||
| let canister_info = CanisterInfo::load(config, canister_name)?; | ||
| let request_id = install_canister(&client, &canister_info)?; | ||
|
|
||
| if args.is_present("async") { | ||
| println!("0x{}", String::from(request_id)); | ||
| } else { | ||
| wait_on_request_status(&client, request_id)?; | ||
| } | ||
| Ok(()) | ||
| } | ||
| Ok(ReadResponse::Rejected { | ||
| reject_code, | ||
| reject_message, | ||
| }) => Err(DfxError::ClientError(reject_code, reject_message)), | ||
| // TODO(SDK-446): remove this matcher when moving api_client to ic_http_agent. | ||
| // `install` cannot return Unknown. | ||
| Ok(ReadResponse::Unknown) => Err(DfxError::Unknown("Unknown response".to_owned())), | ||
| Err(x) => Err(x), | ||
| } | ||
| Ok(()) | ||
| } | ||
| } | ||
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.
Could we rename the function rather than rely on a comment? I haven't looked yet but I assume that
create_canister_idmutatescanister_infoor writes todfx.jsonor both.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.
It writes to
build/<project_name>/canister.id. I'll refactor that.