Blob XML parsing from manual to Serde #176
Conversation
| } | ||
|
|
||
| impl fmt::Display for Etag { | ||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { |
There was a problem hiding this comment.
For now this may be fine, but we need to support weak etags at some point. See https://azure.github.io/azure-sdk/dotnet_introduction.html#conditional-request-methods for some links. We do have sevices that depend on this (e.g. AppConfig).
| let storage_account = | ||
| StorageAccountClient::new_access_key(http_client.clone(), &account, &master_key) | ||
| .as_storage_client(); | ||
| let container = storage_account.as_container_client(&container_name); |
There was a problem hiding this comment.
Would you say "as_" is more idiomatic in Rust? In most other languages, we use "get_" to get a "sub-client". I see "as_" in Rust more for conversations of sort, or is this because StorageAccountClient is all these things as well? We tend to think of getting clients from an account.
There was a problem hiding this comment.
In general in Rust you use as_ for a function that creates a new struct but does not take ownership and is cheap to call. into_ and to_ are used for expensive conversions that do or do not take ownership (respectively).
The difference between as_ and get_ is that, in general, as_ conveys the idea that the whole struct is being used while with get_ you receive only a subset of it.
For example, std::str::String defines both get and as_str. get gives back a substring while as_str gives back the whole string.
In the case of the storage client we are wrapping the whole storage account so I thought as_storage_client was the right name.
The same applies to storage_account.as_container_client(container_name). In my mind it reads as "create a new container client that includes the whole storage account plus the container name".
Maybe someone else can validate my statements, I am by no means an expert on the matter.
There was a problem hiding this comment.
Thanks. That's very informative. I guess I hadn't quite recognized that pattern, but it makes sense the way you explain it (and does jive with what I've seen in my limited experience with Rust).
Conceptually, I see what you mean. At least in .NET (and I suspect other language SDKs) we do create new instances of classes that represent a container from an account, and blob from container, but they should (and do, AFAIK) share a pipeline that is really the expensive part (built-in and user-added policies, HTTP client and it's mockable transports, etc.). That seems to be what you're insinuating here: it's still all that the account uses, but with a different set of methods.
2 participants
This PR tries to "modernize" the blob response parsing from XML.
This was done by replacing the hand rolled XML parsing by using
serdeas much as possible. The bulk of the change is insdk/storage/src/blob/blob/mod.rs.This PR was triggered by issue #175 and should address it by exposing the missing fields to the caller.
This PR also progresses to the goal or removing unnecessary traits and converting the untyped strings into more meaningful types (even though most of the time they are just string wrappers).
I am not particularly happy about the
ListBlobsResponsestruct hierarchy, particularly having to specifyblobstwice to get the vector is ugly. Suggestions are welcome.