Skip to content

Conversation

demoray
Copy link
Contributor

@demoray demoray commented Sep 12, 2023

This is an initial implementation for supporting long running operations. This models the Python SDK for how the state is extracted.

Eventually, this should be wrapped up into a trait that the clients can implement, rather than manually implementing the polling in the generated into_future.

For evaluation, this PR includes generating the following crates:

  • azure-mgmt-keyvault, as this crate uses properties.provisioningState to indicate LRO
  • azure-mgmt-signalr, as this crate uses a combination of Location and Azure-AsyncOperations headers to indicate LRO

This is an initial implementation for supporting `properties.provisioningState` model of long running operations.  This models the Python SDK for how the state is extracted.

Once each of the methods that identifying state are included, thish should be wrapped up into a trait that the clients can implement, rather than manually implementing the polling in the generated `into_future`.
@demoray
Copy link
Contributor Author

demoray commented Sep 12, 2023

Using the example, we get the following log:

[2023-09-12T16:45:36Z DEBUG azure_core::http_client::reqwest] instantiating an http client using the reqwest backend
[2023-09-12T16:45:37Z DEBUG azure_core::policies::transport] the following request will be passed to the transport policy: Request { ... }
[2023-09-12T16:45:37Z DEBUG azure_core::http_client::reqwest] performing request PUT 'https://management.azure.com/...'
[2023-09-12T16:45:37Z DEBUG reqwest::connect] starting new connection: https://management.azure.com/
[2023-09-12T16:45:39Z TRACE azure_core::policies::retry_policies::retry_policy] Successful response. Request=Request { ... }
[2023-09-12T16:45:39Z TRACE azure_mgmt_keyvault::profile_hybrid_2020_09_01::vaults::create_or_update] current provisioning_state: Other("RegisteringDns")
[2023-09-12T16:45:44Z DEBUG azure_core::policies::transport] the following request will be passed to the transport policy: Request { ...}
[2023-09-12T16:45:44Z DEBUG azure_core::http_client::reqwest] performing request PUT 'https://management.azure.com/...'
[2023-09-12T16:45:45Z TRACE azure_core::policies::retry_policies::retry_policy] Successful response. Request=Request { ...}
[2023-09-12T16:45:45Z TRACE azure_mgmt_keyvault::profile_hybrid_2020_09_01::vaults::create_or_update] current provisioning_state: Other("RegisteringDns")
[2023-09-12T16:45:50Z DEBUG azure_core::policies::transport] the following request will be passed to the transport policy: Request {...}
[2023-09-12T16:45:50Z DEBUG azure_core::http_client::reqwest] performing request PUT 'https://management.azure.com/...'
[2023-09-12T16:45:50Z TRACE azure_core::policies::retry_policies::retry_policy] Successful response. Request=Request { ... }
[2023-09-12T16:45:50Z TRACE azure_mgmt_keyvault::profile_hybrid_2020_09_01::vaults::create_or_update] current provisioning_state: Other("RegisteringDns")
[2023-09-12T16:45:56Z DEBUG azure_core::policies::transport] the following request will be passed to the transport policy: Request {...}
[2023-09-12T16:45:56Z DEBUG azure_core::http_client::reqwest] performing request PUT 'https://management.azure.com/...'
[2023-09-12T16:45:56Z TRACE azure_core::policies::retry_policies::retry_policy] Successful response. Request=Request { url: ... }
[2023-09-12T16:45:56Z TRACE azure_mgmt_keyvault::profile_hybrid_2020_09_01::vaults::create_or_update] current provisioning_state: Succeeded

Vault {
    id: Some(...)
}

@demoray demoray marked this pull request as ready for review September 13, 2023 20:12
@demoray
Copy link
Contributor Author

demoray commented Sep 13, 2023

Ffor using testing the Azure-AsyncOperations model of LROs, I implemented an example shown below using the updated azure-mgmt-signalr crate. I have not checked this in, as it requires using a preview tag, rather than the default one. I executed it via:

cargo run --release --no-default-features --features package-2023-06-01-preview --features enable_reqwest --example create_signalr -- $RESOURCE_GROUP $NAME $LOCATION
use azure_identity::AzureCliCredential;
use azure_mgmt_signalr::{
    models::{ResourceSku, SignalRResource, TrackedResource},
    Client,
};
use std::{env::args, sync::Arc};

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    env_logger::init();

    let resource_group = args().nth(1).expect("please specify resource group name");
    let name = args().nth(2).expect("please specify resource name");
    let location = args().nth(3).expect("please specify location");

    let credential = Arc::new(AzureCliCredential::new());
    let subscription_id = AzureCliCredential::get_subscription()?;
    let client = Client::builder(credential).build();

    let mut params = SignalRResource::new(TrackedResource::new(location));
    params.sku = Some(ResourceSku::new("Standard_S1".to_string()));

    let result = client
        .signal_r_client()
        .create_or_update(params, subscription_id, resource_group, name)
        .await?;

    println!("{:#?}", result);

    Ok(())
}

@johnbatty
Copy link
Contributor

Cool!

I suspect that this recent issue may also be due to lack of LRO support:

S: Serialize,
{
let body: serde_json::Value =
serde_json::from_str(&serde_json::to_string(body).ok()?).ok()?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this doing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The simplest way I could come up with to generically get the json path, regardless of the underlying response type was to parse it here, then reparse it by the caller.

My thought is this could eventually be retooled to be a macro or a trait, but this was significantly simpler to get functional.

@demoray demoray merged commit 9d3efdc into Azure:main Sep 18, 2023
@demoray demoray deleted the begin-lro-implementation branch September 18, 2023 00:25
@cataggar cataggar mentioned this pull request Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants