ctaggart / autorust

MIT License
12 stars 4 forks source link

add HttpClientArc & set user-agent #86

Closed ctaggart closed 3 years ago

ctaggart commented 3 years ago

This allows passing in TokenCredential as a reference to more than one OperationConfig. It also adds getters and setters. I'm waiting on https://github.com/Azure/azure-sdk-for-rust/pull/71 to merge.

rylev commented 3 years ago

@ctaggart I'm not sure how I feel about this. This definitely makes OperationConfig more complicated without a ton of benefit. Is there anyway that we can make some of the token credentials Clone so that if you need more than one, we just clone them?

ctaggart commented 3 years ago

@rylev, I feel the same way. I'll see about Clone.

ctaggart commented 3 years ago

@rylev, I tried a few options and with http connection pooling https://github.com/Azure/azure-sdk-for-rust/issues/78, I'm not seeing a way to avoid lifetimes. Here is what this allows.

    let client = &reqwest::Client::new();
    // TODO EnvironmentCredential::new should allow passing in client
    let token_credential = &EnvironmentCredential::new(options);
    let resources_config = azure_mgmt_resources::OperationConfig::new(client, token_credential);
    let vmware_config = azure_mgmt_vmware::OperationConfig::new(client, token_credential);

The downside is that lifetime is required in a function like this:

async fn delete_resource_group<'a>(
    config: &azure_mgmt_resources::OperationConfig<'a>,
    subscription_id: &str,
    resource_group_name: &str,
) -> Result<()> {
    let response = resource_groups::delete(config, resource_group_name, subscription_id).await?;
    println!("delete group: {:#?}", response);
    Ok(())
}
rylev commented 3 years ago

@ctaggart and why not wrap inside of an Rc or Arc? We can do something like this:

struct OperationConfigImpl {
    client: reqwest::Client,
    token_credential: Box<dyn azure_core::TokenCredential>,
    token_credential_resource: String,
    base_path: String,
    api_version: String,
}

#[derive(Clone)]
pub struct OperationConfig(std::sync::Arc<OperationConfigImpl>);

This makes cloning very cheap and avoids any lifetime issues.

ctaggart commented 3 years ago

Going to use move to the http client interface instead.