Conversation
|
TODO: This needs to automatically pick up logging from the (application) service provider, do this after #4493. |
roji
force-pushed
the
DataSource/DependencyInjection
branch
from
d5646f6 to
1bc340f
Compare
|
Done, this is now based off of #4493. |
roji
force-pushed
the
DataSource/DependencyInjection
branch
from
1bc340f to
faaa013
Compare
NinoFloris
left a comment
NinoFloris
left a comment
There was a problem hiding this comment.
Some feedback but it's neat to see it all come together :)
src/Npgsql.DependencyInjection/NpgsqlServiceCollectionExtensions.cs
Outdated
Show resolved
Hide resolved
| }, | ||
| dataSourceLifetime)); | ||
|
|
||
| serviceCollection.Add( |
There was a problem hiding this comment.
Should always prefer the ServiceDescriptor constructor taking implementationType over implementationFactory. Using factories breaks constructor graph optimizations in DI containers (MS.DI has these for instance) due to factories being opaque methods.
There was a problem hiding this comment.
Did not know that... But what are you concretely suggesting here, given that NpgsqlConnection needs to be resolved from the NpgsqlDataSource?
| This registers a scoped `NpgsqlConnection` which can get injected into your controllers: | ||
|
|
||
| ```csharp | ||
| public class MyController : Controller |
There was a problem hiding this comment.
Who still uses controllers /jk
We may want to swap one of these samples out for a minimal api one instead.
There was a problem hiding this comment.
You're right, I'm a bit behind the times when it comes to ASP.NET. Will do.
There was a problem hiding this comment.
@NinoFloris check out the new samples - looks great. The automatic logging integration is kinda magical!
roji
force-pushed
the
DataSource/DependencyInjection
branch
from
62f49e5 to
777bb49
Compare
3 participants
Closes #4503
/cc @ajcvickers