Disks / Snapshots clients in compute crate do not implement `IntoFuture` for 'delete' operation

[andlr]

Documentation for disks::Delete and snapshots::Delete says (same as in other operations like create/update, which implement LRO):

This RequestBuilder implements a Long Running Operation (LRO). To finalize and submit the request, invoke .await, which which will convert the RequestBuilder into a future executes the request and polls the service until the operation completes. In order to execute the request without polling the service until the operation completes, use RequestBuilder::send(), which will return a lower-level Response value.

But both of them do not implement IntoFuture for their RequestBuilders, so only using send() is possible, without waiting for LRO to finish. Seemingly, this applies to all the other clients' Delete operation as well.

For now the only workaround I've found is adding these implementations manually into the generated code.

[krishanjmistry]

First of all, here's a link to the specification that the disks::Delete is generated from. An excerpt of the disks delete method is included below.

"delete": {
  "tags": [
    "Disks"
  ],
  "operationId": "Disks_Delete",
  "description": "Deletes a disk.",
  "parameters": [
    {
      "$ref": "../../../common-types/v1/common.json#/parameters/SubscriptionIdParameter"
    },
    {
      "$ref": "./diskRPCommon.json#/parameters/ResourceGroupNameParameter"
    },
    {
      "$ref": "#/parameters/DiskNameParameter"
    },
    {
      "$ref": "../../../common-types/v1/common.json#/parameters/ApiVersionParameter"
    }
  ],
  "responses": {
    "200": {
      "description": "OK"
    },
    "202": {
      "description": "Accepted"
    },
    "204": {
      "description": "If the disk is deleted, this is an expected error code."
    }
  },
  "x-ms-examples": {
    "Delete a managed disk.": {
      "$ref": "./examples/diskExamples/Disk_Delete.json"
    }
  },
  "x-ms-long-running-operation": true
}   

The code that implements IntoFuture within AutoRust seems to have a limitation that it expects there to be a response type - otherwise it won't generate the code. Link to the code that generates the method.

In the case of the delete methods, there is no response body object - hence it's not generating the code. For comparison, the disk::Update method does implement IntoFuture as there is a response body.

[andlr]

But there's no need for response body, azure-asyncoperation is in HTTP Header. This implementation works for delete:

fn into_future(self) -> Self::IntoFuture {
    Box::pin(async move {
        use azure_core::{
            error::{Error, ErrorKind},
            lro::{
                get_retry_after,
                location::{get_location, get_provisioning_state, FinalState},
                LroStatus,
            },
            sleep::sleep,
        };
        use std::time::Duration;
        let this = self.clone();
        let response = this.send().await?;
        let headers = response.as_raw_response().headers();
        let location = get_location(headers, FinalState::AzureAsyncOperation)?;
        if let Some(url) = location {
            loop {
                let mut req = azure_core::Request::new(url.clone(), azure_core::Method::Get);
                let bearer_token = self.client.bearer_token().await?;
                req.insert_header(
                    azure_core::headers::AUTHORIZATION,
                    format!("Bearer {}", bearer_token.secret()),
                );
                let response = self.client.send(&mut req).await?;
                let headers = response.headers();
                let retry_after = get_retry_after(headers);
                let bytes = response.into_body().collect().await?;
                let provisioning_state = get_provisioning_state(&bytes).ok_or_else(|| {
                    Error::message(
                        ErrorKind::Other,
                        "Long running operation failed (missing provisioning state)".to_string(),
                    )
                })?;
                log::trace!("current provisioning_state: {provisioning_state:?}");
                match provisioning_state {
                    LroStatus::Succeeded => {
                        return Ok(());
                    }
                    LroStatus::Failed => {
                        return Err(Error::message(
                            ErrorKind::Other,
                            "Long running operation failed".to_string(),
                        ))
                    }
                    LroStatus::Canceled => {
                        return Err(Error::message(
                            ErrorKind::Other,
                            "Long running operation canceled".to_string(),
                        ))
                    }
                    _ => {
                        sleep(retry_after).await;
                    }
                }
            }
        } else {
            Ok(())
        }
    })
}

[krishanjmistry]

Sorry, I was unclear. I was trying to point out there is a bug in the auto-generation whereby it's not handling LRO operations that don't have a body!

[johnbatty]

Fyi, I hit the same issue with my azure-devops-rust-api crate, which also uses the autorust code generator. I fixed it here: https://github.com/microsoft/azure-devops-rust-api/pull/204

I'll take a look at porting the fix back to azure-sdk-for-rust.

[andlr]

Fyi, I hit the same issue with my azure-devops-rust-api crate, which also uses the autorust code generator. I fixed it here: microsoft/azure-devops-rust-api#204

I'll take a look at porting the fix back to azure-sdk-for-rust.

From what I see, your implementation is for operations with a single request, like get:

            quote! {
                impl std::future::IntoFuture for RequestBuilder {
                    type Output = azure_core::Result<()>;
                    type IntoFuture = futures::future::BoxFuture<'static, azure_core::Result<()>>;

                    #[doc = "Returns a future that sends the request and returns the parsed response body."]
                    #[doc = ""]
                    #[doc = "You should not normally call this method directly, simply invoke `.await` which implicitly calls `IntoFuture::into_future`."]
                    #[doc = ""]
                    #[doc = "See [IntoFuture documentation](https://doc.rust-lang.org/std/future/trait.IntoFuture.html) for more details."]
                    fn into_future(self) -> Self::IntoFuture {
                        Box::pin(
                            async move {
                                let _rsp = self.send().await?;
                                Ok(())
                            }
                        )
                    }
                }
            }

IntoFuture implementation for delete operations require observing operation status using URL from azure-asyncoperation HTTP header

[johnbatty]

Ah yes, thanks - I missed the comments above that this was a LRO operation. There will need to be some changes to cope with that.