Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Better file support for kv:put #633

Merged
merged 2 commits into from
Sep 20, 2019
Merged

Better file support for kv:put #633

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
19ccc91
Adds support for non-utf8 files for kv:put. Streams files directly fr…
phayes Sep 19, 2019
09cd46e
Removing weird res binding
phayes Sep 19, 2019
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
18 changes: 10 additions & 8 deletions src/commands/kv/key/put.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,26 +41,28 @@ pub fn put(
};
let url = Url::parse_with_params(&api_endpoint, query_params);

let client = http::auth_client(&user);

let url_into_str = url?.into_string();

// If is_file is true, overwrite value to be the contents of the given
// filename in the 'value' arg.
let body_text = if is_file {
let mut res = if is_file {
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 needs to be mutable?

Copy link
Contributor Author

@phayes phayes Sep 20, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Weirdly, it does need to be mutable. (rustc complains if I remove the mut).

On line 73 we have let parsed = res.json();, which needs the response to be mutable. The signature of this method is:

pub fn json<T: DeserializeOwned>(&mut self) -> ::Result<T>

I think this is because when we return the response, the body hasn't been read yet. And if we have an error then we need to read the body, which mutates the response as we're reading it in.

Copy link
Contributor

Choose a reason for hiding this comment

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

This makes sense (and TIL)!

match &metadata(value) {
Ok(file_type) if file_type.is_file() => fs::read_to_string(value),
Ok(file_type) if file_type.is_file() => {
let file = fs::File::open(value)?;
client.put(&url_into_str).body(file).send()?
}
Ok(file_type) if file_type.is_dir() => {
failure::bail!("--path argument takes a file, {} is a directory", value)
}
Ok(_) => failure::bail!("--path argument takes a file, {} is a symlink", value), // last remaining value is symlink
Err(e) => failure::bail!("{}", e),
}
} else {
Ok(value.to_string())
client.put(&url_into_str).body(value.to_string()).send()?
};

let client = http::auth_client(&user);

let url_into_str = url?.into_string();
let mut res = client.put(&url_into_str).body(body_text?).send()?;

if res.status().is_success() {
message::success("Success")
} else {
Expand Down