-
Notifications
You must be signed in to change notification settings - Fork 334
Conversation
…om disk to the endpoint
@phayes Thank you for this PR! This is fantastic. |
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.
one tiny change request :)
// 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 { |
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.
I don't think this needs to be mutable?
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.
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.
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.
This makes sense (and TIL)!
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.
LGTM!
This PR adds better file support for
kv:put