Fix / Add support for ResumeToken from Operation<T> concrete classes

[vhvb1989]

Problem

The current Operations<T> concrete classes from the Azure SDK for C++ on Storage and Keyvault are either aborting (as not implemented) or producing a wring resume token when calling GetResumeToken() from the Operation.

Expected fix

Consider the next statement : DeleteKeyOperation operation = client.StartDeleteKey(...); Where:

• DeleteKeyOperation: The Operation<T> concrete class.
• operation: The operation instance.
• StartDeleteKey(...): The operation API producer from an SDK client.

The operation API producer must have an overload method which takes an std::string const& resumeToken and be able to return the same operation instance which was returned before and was used to produce the resumeToken.

See next code:

DeleteKeyOperation operation = client.StartDeleteKey(name, context);
std::string resumeToken = operation.GetResumeToken();

DeleteKeyOperation identicOperation = client.StartDeleteKey(resumeToken);

// At this point, polling from operation or from identicOperation is the same.

Notes

• The resumeToken requires an SDK client and the same API producer in order to re-create the operation.
• Re-creating the operation from the producer API will send a request to the server to complete the operation init. It needs to take a context.

[vhvb1989]

@JeffreyRichter @RickWinter please review and mention if there's something to be changed.

[Jinming-Hu]

I thought it would be like

DeleteKeyOperation identicOperation(client, resumeToken);

[Jinming-Hu]

But as long as your proposal is approved by working group, I'm happy to do the same for copy operation in storage SDK.

[JeffreyRichter]

Having a StartXxx method overload on the client is the better design and matches other languages.

But now that I'm thinking about it, I see additional problems. I'm tempted to say that this StartXxx overload SHOULD make 1 polling request to the service to populate the RawResponse, Value, and status fields. This is how it would work in real life (without a polling helper class: The token would be sent from 1 PC to another and the other wouldn't know the state of anything; it would have to poll the service once for the client to learn the current status of the LRO.

This means that client.StartDeleteKey(resumeToken); also needs to take a Context parameter since it should make an I/O request now.

[vhvb1989]

Having a StartXxx method overload on the client is the better design and matches other languages.

But now that I'm thinking about it, I see additional problems. I'm tempted to say that this StartXxx overload SHOULD make 1 polling request to the service to populate the RawResponse, Value, and status fields. This is how it would work in real life (without a polling helper class: The token would be sent from 1 PC to another and the other wouldn't know the state of anything; it would have to poll the service once for the client to learn the current status of the LRO.

This means that client.StartDeleteKey(resumeToken); also needs to take a Context parameter since it should make an I/O request now.

@JeffreyRichter what if we get a resumeToken from a completed operation? Then, on reconstruction, it might fail if we try to poll from it and the server doesn't have a record for the operation to answer about.

And also, the original StartXXX function is not calling Poll when it creates the operation. It gets the response from the Server about the operation was created/init and that's all we use to create the operation with the polling implementation to get updates. So, following the same pattern, re-constructing the operation would let the operation in the same exactly state as when the resumeToken was generated. (Basically like some sort of Operation-backup)

[vhvb1989]

After conversation with Azure SDK Core team, the decision is to call Poll when re-constructing the operation to complete the init instead of serializing all the information from the operation in the resumeToken.

[Jinming-Hu]

the decision is to call Poll when re-constructing the operation to complete the init

@vhvb1989 Need clarification here. How exactly is the operation re-constructed?

[JeffreyRichter]

Customer calls StartCopyBlob which calls REST Copy Blob and gets back x-ms-copy-id: and x-ms-copy-status: <success | pending>. StartCopyBlob returns Operation<T>-derived type with Status set to success or pending and has a resume token of . You also need to store a reference to the BlobClient in the Operation<T>-derived type.

Customer calls Poll which calls blobClient.GetProperties and compares the returned x-ms-copy-id with the resumetoken and updates status field. You could also return a result T that has the other values such as x-ms-copy-completion-time:, x-ms-copy-status-description: . x-ms-copy-progress: <bytes copied/bytes total> could be in a field in your derived type (if you want).

Customer then gets the resume token (x-ms-copy-id) and calls an overload of StartCopyBlob passing in a the resume token (id) and an optional Context. This CopyBlob overload constructs an instance of the same Operation<T>-derived type passing the resume token (id) to the ctor and also passing a reference to the same blobClient. Then, StartCopyBlob calls Poll on the Operation<T>-derived type; this ensures that the status and other fields are initialized to some sane values before returning the Operation<T>-derived type back to customer code.

Now the customer can inspect the Operation<T>-derived type and call Poll(UntilDone) like they could before.

[Jinming-Hu]

Customer calls StartCopyBlob which calls REST Copy Blob and gets back x-ms-copy-id: and x-ms-copy-status: <success | pending>. StartCopyBlob returns Operation-derived type with Status set to success or pending and has a resume token of . You also need to store a reference to the BlobClient in the Operation-derived type.

Customer calls Poll which calls blobClient.GetProperties and compares the returned x-ms-copy-id with the resumetoken and updates status field. You could also return a result T that has the other values such as x-ms-copy-completion-time:, x-ms-copy-status-description: . x-ms-copy-progress: <bytes copied/bytes total> could be in a field in your derived type (if you want).

Customer then gets the resume token (x-ms-copy-id) and calls an overload of StartCopyBlob passing in a the resume token (id) and an optional Context. This CopyBlob overload constructs an instance of the same Operation-derived type passing the resume token (id) to the ctor and also passing a reference to the same blobClient. Then, StartCopyBlob calls Poll on the Operation-derived type; this ensures that the status and other fields are initialized to some sane values before returning the Operation-derived type back to customer code.

Now the customer can inspect the Operation-derived type and call Poll(UntilDone) like they could before.

@JeffreyRichter I believe this will work. But no other SDK is doing like this. None of over sdk has an overloading of StartCopy.

[JeffreyRichter]

You can probably get away without implementing this now (and maybe never as most customers won't need this feature for CopyBlob). I do see that .NET has ID (which is the same as our resume token): https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/core/Azure.Core/src/Operation.cs#L22

But, I agree that they did not provide the StartCopy overload to use it.

[Jinming-Hu]

Conclusion from the meeting today: we can also copy what .Net does https://docs.microsoft.com/en-us/dotnet/api/azure.storage.blobs.models.copyfromurioperation.-ctor?view=azure-dotnet#Azure_Storage_Blobs_Models_CopyFromUriOperation__ctor_System_String_Azure_Storage_Blobs_Specialized_BlobBaseClient_

[ahsonkhan]

@tg-msft how do you recreate things in the Operation<T> ctor like RawResponse, etc. from the resume token? Do you send a request to the service on resume? That's what we were thinking by adding an overload to StartCopy that accepts the token, makes a request, and returns a fully hydrated Operation<T> concrete derived class.

If we don't send a service request (which we can't if the token is passed in to the Operation ctor), would we leave the raw response uninitialized? https://github.com/Azure/azure-sdk-for-cpp/blob/1408064c7a02322c696cfa38d927a35f5a16a9ac/sdk/core/azure-core/inc/azure/core/operation.hpp#L36-L37

[tg-msft]

Yes, RawResponse is null if you've never made a request. We think most customers understand that just creating a new instance of a type means we won't have sent a request to the service yet. https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/storage/Azure.Storage.Blobs/src/Models/CopyFromUriOperation.cs#L90

[ahsonkhan]

So does that mean that resuming an Operation<T> in a completed state returns one that is not completed? Why wouldn't we want to retain that state info?

Similarly, do we ignore the status of the original LRO, and leave it unset within the one that get's recreated, and then let the first call to Poll() set the fields to the right values?

[Jinming-Hu]

I think it's fine to leave m_rawResponse null for a re-created operation.

[vhvb1989]

KeyVault updated: https://github.com/Azure/azure-sdk-for-cpp/pull/2093

[github-actions[bot]]

Hi @vhvb1989, we deeply appreciate your input into this project. Regrettably, this issue has remained inactive for over 2 years, leading us to the decision to close it. We've implemented this policy to maintain the relevance of our issue queue and facilitate easier navigation for new contributors. If you still believe this topic requires attention, please feel free to create a new issue, referencing this one. Thank you for your understanding and ongoing support.