[Storage] RetryOptions members unnecessarily readonly

[mjrousos]

• Package Name: @azure/storage-blob
• Package Version: 12.0.0-preview.2
• Operating system: Windows
• nodejs
  • version: 12.8.0
• typescript
  • version: 3.5.3

Describe the bug
This is a minor issue, but there seems to be some inconsistency in the RetryOptions interface.

Its members are all readonly, but equivalent fields in the .NET SDK's RetryOptions type are not, nor are members of other 'options' objects used when setting up pipelines (like ProxyOptions or TelemetryOptions.

Expected behavior
I would have expected RetryOption's properties to be gettable and settable (beyond object creation-time) similar to other options interfaces.

[XiaoningLiu]

@mjrousos It's by desing to be readonly for options. Because underlayer code does't keep a reference to the original retry policy options. See

azure-sdk-for-js/sdk/storage/storage-blob/src/policies/RetryPolicy.ts

Line 86 in c7ce347

constructor(

Is any usage scenario for this requirement?

[mjrousos]

I see. Thanks for explaining, @XiaoningLiu. There's no scenario where this is required since there are simple work-arounds, I just had a use case where I created RetryOptions and wanted other code to have an opportunity to change them before creating the RetryPolicy. This can be done by creating new retry options but it seemed strange that that was necessary for RetryOptions and not for other types used to create a RetryPolicy. I see what you mean about the reference to the options object not being stored, though. It's probably fine to leave this code as it is.

[kurtzeborn]

Closing as it looks like this discussion is complete. @mjrousos, please open a new issue if you have any further questions or find any other bugs in the preview. Thank you for trying it out!

[jeremymeng]

Hi Mike @mjrousos let's embrace immutability using spread operator

> oldOption = { a: 1, b: 2 }
{ a: 1, b: 2 }
> newOptiono = { ...oldOption, b: 3 }
{ a: 1, b: 3 }