-
Notifications
You must be signed in to change notification settings - Fork 248
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
Get blob metadata implementation #152
Conversation
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.
Looks good. Just some small nits.
} | ||
|
||
pub fn get(&self, k: &str) -> Option<Bytes> { | ||
self.0.get(k).map(|b| b.clone()) |
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.
Is there a reason to eagerly clone instead of returning a &Bytes
and letting the user decide? I guess cloning Bytes
is fairly inexpensive....
headers | ||
.iter() | ||
.filter(|header| header.0.as_str().starts_with("x-ms-meta-")) | ||
.for_each(|header| { |
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.
nit: can you deconstruct the header
binding as (key, value)
?
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.
Good idea!
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.
Also it's probably best to move this code to the Metadata
struct.
Fixes #115
This PR:
Done:
get_metatadata()
in the blob client. The function does not require parameters and returns aGetBlobMetadataResponse
with the response headers along with the parsed metadata.Metadata
struct tobytes::Bytes
(previously it wasString
). This is done to better align with HeaderValue. HeaderValue does not force its contains to be a valid str so we do not require it either.ToDo:
Out of scope
Metadata poses limitations on the "key" name (it must adhere to the naming rules for C# identifiers). The SDK does not enforce it. It relies on server sides errors instead.