[Storage] [STG105] Create Session - #5274
vincenttran-msft wants to merge 3 commits into
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 3 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
| endpoint: blob_url, | ||
| version: options.version, | ||
| pipeline, | ||
| session_options: options.session_options, |
There was a problem hiding this comment.
As discussed, I don't think we need to persist this.
There was a problem hiding this comment.
Sounds good. For now I regenerated temporarily off of https://github.com/Azure/azure-rest-api-specs/tree/vincenttran/create_session_swap and for the lowest impact change right now, just added a separate new_with_session() to keep the feature working while not persisting the SessionOptions.
If we want to go down the route of handwriting the options bag, we can do that, but it's abit more mechanical and I want to ensure we want to go down that route before getting the necessary .tsp change in place + handwritten replacement copies of all the options bags.
| apply_client_defaults(client_options); | ||
| let default_session_options = SessionOptions::default(); | ||
| let session_options = session_options.unwrap_or(&default_session_options); | ||
| let per_retry_policies = build_auth_policies( |
There was a problem hiding this comment.
If you pass just options in, you could pass just options here.
There was a problem hiding this comment.
This one was abit confusing and a super subtle difference, so here is the full example:
What happens now (correct):
build_pipelinerunswith_client_defaults(). transport is None, so it constructs T1. The parent pipeline gets T1.build_auth_policiesreceives&client_options, which already holds T1.- In the implicit-provider branch it does
client_options.clone()- so the copy is still T1. (L188 ofmod.rs) - That goes into
BlobServiceClient::new(), which re-enters build_pipeline -> with_client_defaults (L33 ofblob_service_client.rs). This timetransport.is_none()is false (L79 ofmod.rs), so it's left alone. The provider's pipeline also runs on T1. - One HTTP client, shared by the data-plane pipeline and the session-minting client.
What would happen with raw options:
build_pipelinestill makes T1 for the parent pipeline.build_auth_policiesgets the raw options, where transport is still None, and clones that intoBlobServiceClientOptions.- BlobServiceClient::new -> build_pipeline -> with_client_defaults -> transport.is_none() is now true -> constructs T2.
Now if we really wanted to, we could take the generated options as mut and then set the transport so then that transport.is_none() check wouldn't cause the second transport (T2) to be spun up, but want to see your take since a previous comment pushed back on why client_options was mutable, so it feels abit like we are just pushing the &mut up one layer.
2 participants
STG105 Feature - Create Session
.tsp: https://github.com/Azure/azure-rest-api-specs/tree/rust/feature/storage_stg105