-
Notifications
You must be signed in to change notification settings - Fork 19
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
Update geo-redundancy code sample and readme #8
Update geo-redundancy code sample and readme #8
Conversation
@dotnet/docs Requesting review for code sample changes. This repo is being updated to include a sample for the .NET v12 SDK (please see changes within the v12 folder). I'm open to any and all feedback you may have. Thanks so much! |
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.
A few nits, and suggestions.
Co-authored-by: David Pine <david.pine@microsoft.com>
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.
It seems odd that an update that went out of its way to replace instances of "circuit breaker pattern" with "geo-redundancy options" doesn't actually use any of the built-in geo-redundancy options in the v12 library. While v12's built-in options are certainly different than v11's, I'm not sure why we wouldn't be guiding customers towards them, and determining how these might be augmented if we feel they are currently lacking, rather than asking customers to write all this code on their own. It would be good to sync up on the purpose of the v12 sample.
@jaschrep-msft I'm very glad to sync up with you to discuss. I was under the impression that one of the main additions in v12 was the ability to set GeoRedundantSecondaryUri within BlobClientOptions to redirect retries to secondary storage. I included that in the sample, but if I'm missing other options I'm happy to make changes. |
@pauljewellmsft I must have missed it. I saw the explicit secondary uri client and didnt' see the usual options at first glance. Would still love to discuss this sample with you though to get a better idea of what we are guiding customers towards and perhaps what work the library can do for them in this regard. |
…com/pauljewellmsft/storage-dotnet-circuit-breaker-ha-ra-grs into pauljewell-update-redundancy-sample
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.
Thanks for tagging us @pauljewellmsft
This looks great. I only found one small nit. Let's
@jaschrep-msft Sample updated and streamlined per our conversation, if you don't mind taking a look. |
Modified folder structure in the repo to split out the v11 sample from the newly added v12 sample. Updated readme to show different paths for v11 and v12.