Azure / azure-cosmos-dotnet-v3

.NET SDK for Azure Cosmos DB for the core SQL API
MIT License
738 stars 493 forks source link

Do we have support to have CosmosClient included in AddAzureClients #4002

Open Ponant opened 1 year ago

Ponant commented 1 year ago

Is the CosmosClient supported here https://devblogs.microsoft.com/azure-sdk/best-practices-for-using-azure-sdk-with-asp-net-core/

This extension is usefull and neat, it would work like this for Blob Storage

builder.Services.AddAzureClients(options =>
        {
            //Add storage accounts
                options.AddBlobServiceClient(i"blabla").;
            // Use DefaultAzureCredential by default
            options.UseCredential(new DefaultAzureCredential());
        });

I wonder if we can add AddCosmosServiceClient? I did not find it in the docs.

sourabh1007 commented 1 year ago

https://learn.microsoft.com/en-us/dotnet/api/microsoft.extensions.azure?view=azure-dotnet

You can see all the extensions available for azure SDK. If you have specific requirement, please create this issue in azure SDK i.e.https://github.com/Azure/azure-sdk-for-net/

jsquire commented 1 year ago

@sourabh1007: The extensions are built on interfaces from Azure.Core and must be implemented as extension methods in the package where the associated client is defined. This would need to be added in this repository and in the Cosmos package directly. I'm happy to discuss further; please feel free to reach out directly.

Examples:

kirankumarkolli commented 1 year ago

@jsquire do above extensions gate on Track2 libraries (ex: Azure.* namespace) only?

CosmosDB Test2 is not yet available.

jsquire commented 1 year ago

@jsquire do above extensions gate on Track2 libraries (ex: Azure.* namespace) only?

CosmosDB Test2 is not yet available.

@kirankumarkolli: Not at all, you're welcome to use them. There are no gates, just the ability to extend the Azure.Core interfaces, which should be available to v3 of the Cosmos package due to the existing dependency. Developers referencing Microsoft.Extensions.Azure and the Cosmos client library would see the extensions and have access to the shortcut for registering Cosmos clients with DI. Developers not using the Cosmos package would not see those extensions.

There are some assumptions about patterns - such as using TokenCredential for auth - but you have control over which extensions you make and can either work around them or choose to not include that overload.

sourabh1007 commented 1 year ago

@jsquire Thanks for the explanation. @Ponant Right now, these extension methods are not available in cosmosdb SDK. We are adding it in our backlog items. Will cover it in near future.

Ponant commented 1 year ago

Thank you. It will be good to give the option of not using NewtonSoft's json package. I am currently using this

builder.Services.TryAddSingleton<CosmosClient>(sp =>
new CosmosClientBuilder(accountEndpoint: accountEndpoint, authKeyOrResourceToken: authKeyOrResourceToken)
.WithSerializerOptions(new CosmosSerializationOptions { PropertyNamingPolicy = CosmosPropertyNamingPolicy.CamelCase })
.Build());

And can make it work with DefaultAzureCredentials (notice how the credentials are invoked above independent from the service used):

options.UseCredential(new DefaultAzureCredential());

cago-rebl commented 1 year ago

Hi,

Is there any time range on when this feature would be able to use? Would be a really good option to use.

levimatheri commented 9 months ago

@Ponant / @sourabh1007 / @jsquire isn't it sufficient to do the following?

services.AddAzureClients(clientBuilder =>
{
     clientBuilder.AddClient<CosmosClient, CosmosClientOptions>((options, credential, provider) =>
     {
    var cosmosDbOptions = provider.GetRequiredService<IOptions<CosmosOptions>>().Value;
    return new CosmosClientBuilder(
                cosmosDbOptions.Endpoint,
                credential)
            .WithApplicationName("[appname]")
            .Build();
    });

    clientBuilder.UseCredential(new DefaultAzureCredential());     
}

I'm also happy to take this up and implement the extension specific to CosmosDb :)

smolleman commented 8 months ago

Haven't tried it but it looks sufficient. I would still love an extension method to make registering cosmos a one-liner (or perhaps a two-liner).

kirankumarkolli commented 8 months ago

@Ponant / @sourabh1007 / @jsquire isn't it sufficient to do the following?

services.AddAzureClients(clientBuilder =>
{
     clientBuilder.AddClient<CosmosClient, CosmosClientOptions>((options, credential, provider) =>
     {
  var cosmosDbOptions = provider.GetRequiredService<IOptions<CosmosOptions>>().Value;
  return new CosmosClientBuilder(
              cosmosDbOptions.Endpoint,
              credential)
          .WithApplicationName("[appname]")
          .Build();
    });

    clientBuilder.UseCredential(new DefaultAzureCredential());     
}

I'm also happy to take this up and implement the extension specific to CosmosDb :)

@levimatheri it would be a great addition. Can you please propose the config section contract also? We are very happy for contributions, please let us know if-any is needed from our side.

/cc: @jcocchi

jsquire commented 8 months ago

@Ponant / @sourabh1007 / @jsquire isn't it sufficient to do the following?

services.AddAzureClients(clientBuilder =>
{
     clientBuilder.AddClient<CosmosClient, CosmosClientOptions>((options, credential, provider) =>
     {
  var cosmosDbOptions = provider.GetRequiredService<IOptions<CosmosOptions>>().Value;
  return new CosmosClientBuilder(
              cosmosDbOptions.Endpoint,
              credential)
          .WithApplicationName("[appname]")
          .Build();
    });

    clientBuilder.UseCredential(new DefaultAzureCredential());     
}

I'm also happy to take this up and implement the extension specific to CosmosDb :)

You do NOT want to call clientBuilder.UseCredential(new DefaultAzureCredential()); This will override the global default credential created for all registered Azure SDK clients. Other than that, it looks reasonable to me.

levimatheri commented 8 months ago

@kirankumarkolli here's the config contract I'm thinking for the different options of creating CosmosClient.

{
  // Using TokenCredential
  "CosmosDb": {
    "AccountEndpoint": "https://XXXXX.documents.azure.com:443/"
  },
  // Using connection string
  "CosmosDb": {
    "ConnectionString": "AccountEndpoint=https://XXXXX.documents.azure.com:443/;AccountKey=SuperSecretKey;"
  },
  // Using auth key or resource token
  "CosmosDb": {
    "AccountEndpoint": "https://XXXXX.documents.azure.com:443/",
    "AuthKeyOrResourceToken": "SuperSecretKey"
  },
  // Using azure key credential
  "CosmosDb": {
    "AccountEndpoint": "https://XXXXX.documents.azure.com:443/",
    "AuthKeyOrResourceTokenCredential": {
      "Key": "SuperSecretKey"
    }
  }
}
kirankumarkolli commented 8 months ago

The configuration gets passed through into the AzureClientFactoryBuilder.RegisterClientFactory which in-turn calls ClientFactory.CreateClient. The names maps to the constructor parameter names.

https://github.com/Azure/azure-sdk-for-net/blob/9498255f3bc30996f20ea18494535f0b3b387df2/sdk/extensions/Microsoft.Extensions.Azure/src/AzureClientFactoryBuilder.cs#L35 https://github.com/Azure/azure-sdk-for-net/blob/9498255f3bc30996f20ea18494535f0b3b387df2/sdk/extensions/Microsoft.Extensions.Azure/src/Internal/ClientFactory.cs#L22

/cc: @jcocchi , @ealsur thoughts/feedback?