[BUG][Stroage mgmt Track2] Cannot get BlobRestoreStatus object from RestoreBlobRanges 202 response

[yifanz0]

Library name and version

Azure.ResourceManager.Storage 1.0.0-beta.9

Describe the bug

After calling RestoreBlobRanges, as required by the feature team, we need to print the Restore status in Powershell console as soon as we get the first 202 response from the server. With Track2 SDK, we're not able to easily get the Restore status that's in the response. That being said, we cannot get the BlobRestoreStatus object which is supposed to be the output of RestoreBlobRanges, from the 202 response.

Expected behavior

When the server returns 202 response, PowerShell should be able to get the BlobRestoreStatus object from the response directly.
It's expected that in Track2 the behavior here should be aligned with that of Track1. The code for Track1 can be found here You can find the PSH code for this API based on track1 SDK here.

Actual behavior

Currently there's no easy way for PowerShell to get the RestoreId. Instead, the workaround that we're using right now is to fetch the raw response returned by the api call directly. And then parse the response to get the RestoreId.
Dictionary<string, object> temp = (Dictionary<string, object>)restoreLro.GetRawResponse().Content.ToObjectFromJson();

Also, there's no way to get BlobRestoreStatus object.

Reproduction Steps

Call RestoreBlobRanges with WaitUtil.Started. We cannot get BlobRestoreStatus from restoreLro.

var restoreLro = account.RestoreBlobRanges(
                        WaitUntil.Started,
                        new Track2Models.BlobRestoreContent(
                            this.TimeToRestore,
                            ParseBlobRestoreRanges(this.BlobRestoreRange))
                        );

Environment

.NET SDK (reflecting any global.json):
Version: 6.0.300
Commit: 8473146e7d

Runtime Environment:
OS Name: Windows
OS Version: 10.0.19044
OS Platform: Windows
RID: win10-x64
Base Path: C:\Program Files\dotnet\sdk\6.0.300\

Host (useful for support):
Version: 6.0.5
Commit: 70ae3df4a6

.NET SDKs installed:
3.1.419 [C:\Program Files\dotnet\sdk]
6.0.105 [C:\Program Files\dotnet\sdk]
6.0.203 [C:\Program Files\dotnet\sdk]
6.0.300 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
Microsoft.AspNetCore.All 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.App 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 3.1.25 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 6.0.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 3.1.25 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 3.1.25 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 6.0.5 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET runtimes or SDKs:
https://aka.ms/dotnet-download

[azure-sdk]

Label prediction was below confidence level 0.6 for Model:CategoryLabels: 'Mgmt:0.56927365,Client:0.42848876,Service:0.0017983767'

[jsquire]

Thank you for your feedback. Tagging and routing to the team member best able to assist.

[m-nash]

It seems that when Track 2 LRO was designed there wasn't a mechanism for retrieving the interim status inside the LRO object itself other than "InProgress" and "Complete/Failed". I can see us taking a couple different approaches here.

Option 1

We could add an extension method to ArmOperation<BlobRestoreStatus> which would basically deserialize the raw response into a BlobRestoreStatus model and return that.

public BlobRestoreStatus GetStatus(this ArmOperation<BlobRestoreStatus> operation)

We would need to provide a sync and async version of this since it would be reading the response content stream.

Option 2

We could add a configuration in swagger to mark an LRO as using the same model for interim status as the final state. In most LROs this isn't the case but when it is if we knew this would could allow the Value property to return the current status and not throw an exception. A user would still need to inspect HasCompleted to know the LRO had finished.

Option 3

We could expand the Operation definition to include a new type Operation<T, U> where T is the final state model and U is the interim state model. This is a much more generic way to do option 2, however I don't know of any actual use case that would need this right now.

[tg-msft]

In data-plane, we do this by customizing the class derived from Operation<T>. That's closest to Option 1 (not sure if you need the sync/async split since any operation you'd do this for should be buffered?). I don't think Option 3 is the answer (this LRO looks like a RELO rather than a Status Monitor). I don't know that we have prior art for Option 2. It's not a bad idea for RELOs in general, though I'm not sure we have enough metadata to do it automatically.

[m-nash]

@tg-msft yeah we decided against custom types for LRO in mgmt since it was thousands of types. Our mechanism was to do the extension method if it was an ArmOperation since we could isolate the scope to a specific T and if it was an ArmOperation then we would add a new method.

For option 2 there isn't enough metadata today you are correct this would be a new param in swagger so users would need to explicitly flag these in this way.

[blueww]

@m-nash , @tg-msft,

From storage side, all the 3 options looks good to me.
Hope we can get a fix release soon.

[JoshLove-msft]

I agree with @tg-msft that this Restore Blob Ranges LRO doesn't match the typical LRO pattern, but is more of a RELO. The T in the ArmOperation<T> is BlobRestoreStatus so it is sort of mirroring what the Operation subtypes are meant to do. For this reason, it would probably be awkward to subtype ArmOperation<T> as you would end up with two very similar statusey types. It would probably be better to turn BlobRestoreStatus iself into a subtype of ArmOperation so the signature ends up looking like:

public virtual BlobRestoreOperation RestoreBlobRanges(WaitUntil waitUntil, BlobRestoreContent content, CancellationToken cancellationToken = default)

and we have defined:

public class BlobRestoreOperation : ArmOperation

With this approach we wouldn't be adding any net new types so it seems like it would address @m-nash's concern around introducing additional types.

[JoshLove-msft]

@tg-msft thoughts on the suggested approach?

[tg-msft]

It partly depends on how common this is. If a lot of ARM LROs are RELOs with with observable partial status, then we probably want to bake this into ArmOperation<T>'s implementation somehow (basically option 2 above). If this is truly a one off, then the custom LRO makes sense but I'd defer to @m-nash on whether it should be done by hand or created by the generator.

[m-nash]

I prefer Option 2 here since that will definitely isolate the extension method to a single method vs adding it to any LRO which happens to share the same final state model. In this particular case its 1 method either way but that won't always be the case.

@tg-msft any objections?

[m-nash]

@JoshLove-msft not sure I follow the suggestion. If the idea is to mirror more closely what dataplane does in this one instance wouldn't we have this?

public class RestoreBlobRangesOperation : ArmOperation<BlobRestoreStatus>

This is definitely an option its kind of a twist on Option 1 but since we have not GAd this library we still have the option to actually subclass in this case.

Also if we go this route I would like to define what do we do if this was GA? Would we then go with Option 2?

[JoshLove-msft]

It didn't seem to me that BlobRestoreStatus contained anything beyond operation status information that would always be available throughout the lifetime of the operation. This is why I didn't think it needed to be an Operation<T>.

[m-nash]

oh yeah it definitely has unique properties to it which is why this issue was reported since getting those properties requires the customer to manually deserialize themselves

[JoshLove-msft]

I understand that it has unique properties, but aren't all of the properties always available even when the operation is in progress?

I.e. it isn't like other operations that contain status info, and then a final result model that contains the data.

[m-nash]

Spoke offline to @tg-msft and @JoshLove-msft I think we are going to go with option 2 given there are a few of these cases out there and that will solve all of them with generated code so no custom code needed when we come across these cases.