-
Notifications
You must be signed in to change notification settings - Fork 162
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
V4 Connectors usage pattern is too intrusive #1310
Comments
Yes, one of the goals of the rewrite was to unify the API shape for single and multiple connections. If built today, I'd probably consider keyed services instead. But if done so, several features that the new connectors provide wouldn't be possible. For example, have an unnamed local connection string get merged with a named one on CloudFoundry. This was considered a key requirement, because developers typically don't know the CF service binding name upfront. Aside from that, it requires dropping support for .NET 6. Would you consider that problematic? |
Considering .NET 6 is going to go out of support in November, it's not worth dragging that anchor on our necks. On a philosophical level, I disagree with this unification approach. Many other libraries including IOC ones provide the ability to inject things both. For example, Autofac will inject dependencies normally, but when multiple registrations of the same type are present one can instead inject As far as binding names are concerned, I'm not sure I agree with what you said. I think it's reasonable for app to expect bindings to have specific names when it needs to differentiate between two services of same type. Binding name is an optional parameter that can be passed to Cloud Foundry (and mandatory on Tanzu Platform). The effect of this change on adoption must be given serious weight before this is finalized. |
As we discussed last week, it's already possible to register connection objects directly, if desired. For example: builder.Services.AddTransient<NpgsqlConnection>(provider =>
{
var factory = provider.GetRequiredService<ConnectorFactory<PostgreSqlOptions, NpgsqlConnection>>();
var connector = factory.Get();
return connector.GetConnection();
}); While I value your feedback, we discussed and iterated over the current design within the team a year ago, which is when I built the new connectors. Our current priority is to finalize Steeltoe v4. We'll consider adding this to Steeltoe after the initial v4 release, if we get more pushback. I'm leaving this issue open to see if more people would prefer this simplification. |
Problem
As part of Steeltoe v4, it seems there's been a change in how connectors must be consumed. Instead of being able to inject native connection objects of libraries with preconfigured connection strings (how it was done traditionally), it now requires consumers to inject something similar to
ConnectorFactory<RabbitMQOptions, IConnection>
. This is very intrusive as it woulda) require major refactoring to existing apps in potentially hundreds of classes
b) hinder adoption, as customers are reluctant to take on intrusive dependencies. Previously one could argue that steeltoe inclusion was only 1-2 lines of code in startup code that only preconfigures default parameters on objects they already use throughout their code.
Proposed solution
While I suspect the motivation is to support multiple named instances of the same service bindings, both syntaxes should be supported. Easy by default for simple scenarios, - advanced syntax for complex use cases. I would expect to be able to inject raw library connection objects with preconfigured connection strings.
The text was updated successfully, but these errors were encountered: