[Storage] Flattened Client Refactor#3176
Conversation
|
@vincenttran-msft is there a reason PRs with similar titles keep getting created and closed? Everything the PRs do you can do locally e.g., |
Sorry for that. I am not concerned about using these for CI pipelines or Analyze checks, these are because there have been some design decisions that we have been discussing internally on the DevEx side, and so I had both proof-of-concepts in two separate branches and we were reviewing them side by side. Having them up in draft PRs was what I thought was easiest to allow reviewers to pre-read at their leisure and also take a look separately if they weren't as concerned with what was being projected during our screenshare. I prematurely closed one of them thinking we were going with another design, but after our meeting yesterday we ended up going back in the other direction. |
|
You can reopen PRs. |
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
|
@LarryOsterman Hi Larry, any ideas on the test failures in azure_messaging_eventhubs_checkpointstore_blob? Here is the beginning of the relevant portion of the logs: The main change that would affect your codepath is that pub fn new(
endpoint: &str,
container_name: String,
credential: Arc<dyn TokenCredential>,
options: Option<BlobContainerClientOptions>,
) -> Result<Self> {to pub fn new(
endpoint: &str,
container_name: &str,
credential: Option<Arc<dyn TokenCredential>>,
options: Option<BlobContainerClientOptions>,
) -> Result<Self> {Namely:
I did go ahead and update the relevant call-sites to get everything compiling nicely, but I seem to have broke the unit tests in the process 😞 Thanks! |
…t since can't generate recording, add changelog entry
sdk/storage/azure_storage_blob/src/clients/append_blob_client.rs
Outdated
Show resolved
Hide resolved
sdk/storage/azure_storage_blob/src/clients/blob_service_client.rs
Outdated
Show resolved
Hide resolved
sdk/storage/azure_storage_blob/src/clients/blob_service_client.rs
Outdated
Show resolved
Hide resolved
heaths
left a comment
There was a problem hiding this comment.
Apart from my one overarching comment about idiomacy, the rest of the changes look great!
sdk/storage/azure_storage_blob/src/clients/append_blob_client.rs
Outdated
Show resolved
Hide resolved
sdk/storage/azure_storage_blob/src/clients/blob_service_client.rs
Outdated
Show resolved
Hide resolved
heaths
left a comment
There was a problem hiding this comment.
Code is fine, but not sure why you downgraded package specs. We've been specific to date.
.tsp: Azure/azure-rest-api-specs#37830from_urlmethods to support client construction directly from URLs, as well as specifically URLs already containing SAS parameters