dotnet / dotnet/efcore

Allow users to pass in CosmosClient directly to UseCosmos()

Open
#35,482 10 comments 4 reactions 0 assignees View on GitHub
area-cosmos customer-reported needs-design
Dominant language
C#
Stars
14.8k
Forks
3.4k
PR merge metrics
PR metrics pending

Description

This tracks adding the ability to pass a CosmosClient directly to UseCosmos(), allowing users to configure their CosmosClient outside of EF Core, with whatever options the Cosmos SDK provides. This removes the current duplication we have around Cosmos configuration options, where we lift and copy Cosmos SDK options to EF's Cosmos options (inevitably falling behind).

Note that the CosmosClient is a singleton service in EF's internal service provider. When a CosmosClient is passed in via the new API, EF will perform reference equality on it. In other words, as long as the same CosmosClient instance is passed in by the user, a single internal service provider will be used; the moment a different instance is passed in, a new service provider will be created by EF. This means that users need to create the CosmosClient instance outside of EF and store it somewhere (e.g. as a singleton DI service or static member), and then pass that instance. If they e.g. instantiate CosmosClient within the DbContext's OnConfiguring, a different instance will be constructed each time; this will quickly trigger EF's "too many service providers" error, letting the user know they're setting things up incorrectly.

---
# Previous design notes

Cosmos supports passing configuration to CosmosClient via [CosmosClientOptions](https://learn.microsoft.com/en-us/dotnet/api/microsoft.azure.cosmos.cosmosclientoptions?view=azure-dotnet). EF doesn't currently allow to be passed directly by the user - we instead wrap those options, exposing EF mechanisms for specifying them instead. Unsurprisingly, new options have been introduced on CosmosClientOptions which we haven't kept up with; there's general consensus in the team this isn't a great story, and that some mechanism for passing CosmosClientOptions to UseCosmos() makes sense.

The main reason EF wraps CosmosClientOptions options is that CosmosClientOptions doesn't currently support value equality, and so cannot be compared in EF's internal service provider cache - see #35481 for a more general discussion of that problem (which also affects other providers). One way forward on that is https://github.com/Azure/azure-cosmos-dotnet-v3/issues/4965, which would add value semantics to CosmosClientOptions, and so we could use our normal caching mechanism as usual (but that wouldn't help the general problem in #35481, e.g. for other providers).

Even if that isn't implemented, we could still add an API accepting CosmosClientOptions, and do one of two things:
1. Simply ignore CosmosClientOptions in the options caching. As long as users don't vary the configuration (the vast majority?), everything works fine. If users *do* vary, later options get ignored and the first CosmosClient gets picked up (bad).
2. Use reference identity for the caching. Everyone using AddDbContext/OnConfiguring will immediately get "too many service providers" if they use AddDbContext/OnConfiguring (since the instance is always different).

Once we introduce such a mechanism, we could also optionally obsolete the current wrapping options, pointing users to the new API instead.

/cc @Pilchie @NinoFloris

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.