-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Very limited DI registration options, creates a paradox between eager, and post-build services. #78
Comments
@ApacheTech thanks for the detailed Issue. This sounds like something related to the DI framework itself, not particular to this library. Basically the scenario is, how do you register components that require each other. Looking at your code, it seems the issue would be resolved by reducing the code to:
The restrictions are related to the Dependency Injection framework you use itself. A DI framework is really not required to use this library, you could use it in a console application. |
That's pretty much exactly what I've ended up doing. It just seems really hacky. I was suprised there was no out-of-the-box support for it. internal class CosmosCacheRegistrar : IServiceRegistrar
{
public void Register(IServiceCollection services, IConfiguration configuration)
{
services.Configure<CosmosCacheConfiguration>(configuration.GetRequiredSection("CosmosCache"));
services.AddSingleton<IDistributedCache, CosmosCache>(sp =>
{
var config = sp.GetRequiredService<IOptions<CosmosCacheConfiguration>>().Value;
var tokenCredential = sp.GetRequiredService<TokenCredential>();
var options = new CosmosCacheOptions
{
ClientBuilder = new CosmosClientBuilder(config.AccountEndpoint, tokenCredential),
ContainerName = config.ContainerName,
DatabaseName = config.DatabaseName
};
return new CosmosCache(options);
});
}
} As a feature request, it would be nice to have a more fluent builder to set a lot of this stuff. The samples within this repo are outdated, still using |
Thanks for your answer. Let me reiterate a point: This library has no dependency on a specific DI framework, you could use the library without any DI framework. There is no dependency of the library on, for example in this case, Microsoft.Extensions.DependencyInjection, so even if we wanted to, we could not add new extension or shortcuts to make this wire further, we cannot depend on any type of API that is related to this package. When looking at the documentation of this DI framework (https://learn.microsoft.com/en-us/dotnet/core/extensions/dependency-injection), I see that that's basically the way where you coordinate multiple registered components that require coordination among themselves. The limitations or lack-of-thereof of a particular DI framework, while great feedback is nothing we can really process it in this repo. Thanks for the feedback on the samples, I understand they might not cover the whole universe of scenarios, we could add one for a similar scenario that you described. Regarding the |
Additionally, looking at the official Distributed caching docs: https://learn.microsoft.com/en-us/aspnet/core/performance/caching/distributed?view=aspnetcore-8.0 And looking at the source code of other providers, like Redis: The extension methods are pretty similar. |
I think I may have been misunderstood, in part. There is never a requirement for any library to use any IOC container, or any specific DI framework. That said, you do provide minimal support for In a way, using an IOC makes things more complicated than not, because you have the two-stage registration process, and you can't go back to the first, once the container is built. When it was written, how did your team invisage people being able to use the I think my main point is that I don't feel that it should be necessary to have to decompile your library, and rip the guts out the extension method you provide, just to be able to use default Azure credential tokens with CosmosDB. Out of the box, your library provides no easy, or elegant solution, so each project that uses tokens and/or app settings will need to hack their own solution. We can't even leave the |
Thanks again for your feedback.
It's possible to simply create the TokenCredential within the
or if you want the TokenCredential to be used with other components in the same app:
There are a lot of possible scenarios, such as, creating a When looking at the APIs available when registering an Extension: Microsoft.Extensions.Caching.Cosmos/src/CosmosCacheServiceCollectionExtensions.cs Lines 38 to 48 in 6fea577
There is a possibility to access the If you believe there is a better way to do the wiring, by all means, please send a PR and we'll review and incorporate it to the next version. |
One idea could be to use the |
There seems to be no way to use the Options pattern, and default azure credentials.
I have a section in
appSettings.json
as such:I'm registering these within the DI, using
CosmosCacheConfiguration
as a POCO:We also use default azure tokens:
However, all of this requires me to late-bind the cosmos cache, but your package can only be eagerly bound, meaning I cannot get to any of the values required to add cosmos, with what's available out of the box:
If I try to create my own registrar, then have no way of creating an
IOptions<CosmosCacheOptions>
, orIOptionsMonitor<CosmosCacheOptions>
, which are the only two available constructors forCosmosCache
.So we have a paradox in which we can't eager-load the service because we have no access to any of the properties that the service needs; and we can't load the service via an implementation factory, because we can't add to the service collection a posterori.
Has this been resolved any other way? Or, is it possible to get a more robust registration procedure? There may be a more obscure way of resolving this with the package as it stands, but I can't see the path to making it work, with what I currently understand about M.E.DI.
The text was updated successfully, but these errors were encountered: