Policy Architecture #204
Policy Architecture #204
Conversation
Perhaps not in this PR, but we do want to allow for devs to pass in their own policies as well. They've been used for proxying data for some request methods, redirecting to test environments, and likely more. |
| } | ||
| } | ||
|
|
||
| pub struct Response { |
There was a problem hiding this comment.
In our various SDKs we also have Response<T> or something like that, which clients would return to wrap the actual response model, but with response information, e.g.:
Response<KeyVaultKey> response = client.GetKey("name");
KeyVaultKey key = response.Value;
Response rawResponse = response.GetRawResponse();Is this something we should do now, or can we later?
Also, is inner just the body, or the entire response? Might be good to call it body if the former.
There was a problem hiding this comment.
How other SDKs handle returning the stream? Without reading the stream first it is impossible to convert it into a struct (KeyVaultKey in your example). Is there a specific exception for blob storage?
|
|
||
| #[derive(Copy, Clone, Debug)] | ||
| pub struct RetryOptions { | ||
| num_retries: usize, |
There was a problem hiding this comment.
At some point we should respect headers sent by the server, for e.g. in .NET (probably same across languages):
private const string RetryAfterHeaderName = "Retry-After";
private const string RetryAfterMsHeaderName = "retry-after-ms";
private const string XRetryAfterMsHeaderName = "x-ms-retry-after-ms";There was a problem hiding this comment.
BTW: There's a lot of good policies in that folder we should eventually port. Most SDK languages have these or something like them.
There was a problem hiding this comment.
Definitely, the policies as they are, are really just proofs of concept. We'll need to improve them considerably.
| /// TODO | ||
| pub fn with_pipeline( | ||
| account: String, // TODO: this will eventually be a URL | ||
| auth_token: AuthorizationToken, |
There was a problem hiding this comment.
At some point, this should be credential of type TokenCredential. We have a whole pluggable system on this. Some TokenCredential providers just wrap a field and supply that to a policy, like shared key credentials (api-key, etc.). See https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/core/Azure.Core/src/AzureKeyCredential.cs for an example.
There was a problem hiding this comment.
Yes good point. Right now the authorization is tightly coupled with the client. We should extract the code and put it in a specific policy.
| pub struct Request { | ||
| inner: http::Request<bytes::Bytes>, | ||
| } |
There was a problem hiding this comment.
As discussed yesterday in our meeting, this will be an enum accepting either a stream (AsyncRead+AsyncSeek) or a bytes::Bytes. We should merge this as it is now and build upon it.
| } | ||
| } | ||
|
|
||
| pub struct Response { |
There was a problem hiding this comment.
How other SDKs handle returning the stream? Without reading the stream first it is impossible to convert it into a struct (KeyVaultKey in your example). Is there a specific exception for blob storage?
| /// TODO | ||
| pub fn with_pipeline( | ||
| account: String, // TODO: this will eventually be a URL | ||
| auth_token: AuthorizationToken, |
There was a problem hiding this comment.
Yes good point. Right now the authorization is tightly coupled with the client. We should extract the code and put it in a specific policy.
This pull request introduces a new way to architect SDKs, known from here on as the "policy" architecture. This is the architecture that other Azure SDKs use.
Why this architecture?
There are several ways to architect an SDK, but when in doubt it's best to align ourselves closely with the SDKs from other languages. Luckily this architecture lends itself fairly well to Rust. It is our eventual hope that this crate becomes officially supported by the the Azure SDK team, and this is a good faith step in that direction since the SDK team strongly wishes that their SDKs have a similar architecture and only differ from each other when there is a good language specific reason to (and not "just because").
Highlights of the architecture
The architecture is fairly straightforward. At a high-level operations against the Azure API are performed by creating sending requests and their subsequent responses through a pipeline of policies. Each policy in the pipeline can transform the outgoing request and react to incoming responses. It is up to each policy to continue the policy pipeline unless it explicitly chooses not to.
At the end of the pipeline is the
TransportPolicywhich "performs" the HTTP request (either by actually making a network request or in the case of testing, returning some stubbed response object).As an example of a policy, the
Retrypolicy will send a request further down the pipeline keeping track of a specified number of retries. if the response is successful, it returns the response to the caller, and it if is not, it will retry the request until the number of retries has been exhausted.Pipelines are a per-client configuration so basically anything that would make sense to configure for a given client (for all their operations), will be a policy. Operation specific options are based to each operations method.
The implementation so far
The PR (as first submitted up to 22fafa2 implements one operation using this new architecture - the CreateDatabase operation. It should therefore hopefully be easy to see how the architecture will work at a high-level. We'll continue to tweak the specific as we go, but we already have sign off from the SDK team that this is the right direction to head in.
Notes
RequestandResponsetypes specific to the Azure SDK. Currently these are just wrappers aroundhttpcrate types but this might eventually change. Having our own types allows for future flexibility.operationsmodule which will replace both therequestsandresponsesmodules grouping everything together by operation.