Skip to content
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

Change ILoggerFactory internal lifetime to scoped #14698

Closed
ajcvickers opened this issue Feb 13, 2019 · 0 comments
Closed

Change ILoggerFactory internal lifetime to scoped #14698

ajcvickers opened this issue Feb 13, 2019 · 0 comments
Assignees
Labels
breaking-change closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. providers-beware type-enhancement
Milestone

Comments

@ajcvickers
Copy link
Member

This has the following advantages:

  • It eliminates internal service provider explosion when calling UseLoggerFactory with a new instance each time. This is currently a significant pit of failure that has consumed many victims.
  • This also means that the testing pattern of multiple external service provides (e.g. one per test) does not cause the internal cache to explode.
  • It allows a simple implementation for simple logging APIs, since each context instance can naturally have its own logger if desired.

The disadvantages are:

  • Internal singleton services cannot depend directly on a logger from D.I. This means either:
    • Making the service scoped
    • Flowing in the logger from a scoped service, which in turn may require breaking changes to add new logging
@ajcvickers ajcvickers self-assigned this Feb 13, 2019
@ajcvickers ajcvickers added this to the 3.0.0 milestone Feb 13, 2019
ajcvickers added a commit that referenced this issue Feb 14, 2019
Issue #14698

Main changes:
* ILoggerFactory is still captured in AddDbContext, but as a wrapped scoped factory
  * Same thing can be done with UseLoggerFactory
  * No longer pathological to pass a different instance each time
* IMemoryCache is no longer captured from AddDbContext
  * Can still be passed to UseMemoryCache
* IRawSqlCommandBuilder and IMigrationsSqlGenerator have been made scoped
* Validation and query compilation, method call translators, and a few more now accept the logger to use
ajcvickers added a commit that referenced this issue Feb 14, 2019
Issue #14698

Main changes:
* ILoggerFactory is still captured in AddDbContext, but as a wrapped scoped factory
  * Same thing can be done with UseLoggerFactory
  * No longer pathological to pass a different instance each time
* IMemoryCache is no longer captured from AddDbContext
  * Can still be passed to UseMemoryCache
* IRawSqlCommandBuilder and IMigrationsSqlGenerator have been made scoped
* Validation and query compilation, method call translators, and a few more now accept the logger to use
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Feb 15, 2019
ajcvickers added a commit that referenced this issue Feb 15, 2019
Issue #14698

Main changes:
* ILoggerFactory is still captured in AddDbContext, but as a wrapped scoped factory
  * Same thing can be done with UseLoggerFactory
  * No longer pathological to pass a different instance each time
* IMemoryCache is no longer captured from AddDbContext
  * Can still be passed to UseMemoryCache
* IRawSqlCommandBuilder and IMigrationsSqlGenerator have been made scoped
* Validation and query compilation, method call translators, and a few more now accept the logger to use
@ajcvickers ajcvickers modified the milestones: 3.0.0, 3.0.0-preview3 Feb 22, 2019
roji added a commit to roji/efcore.pg that referenced this issue Mar 7, 2019
Also, ModificationCommandBatch now has a dependencies type

See dotnet/efcore#14698
roji added a commit to roji/efcore.pg that referenced this issue Mar 7, 2019
Also, ModificationCommandBatch now has a dependencies type

See dotnet/efcore#14698
roji added a commit to roji/efcore.pg that referenced this issue Mar 8, 2019
Also, ModificationCommandBatch now has a dependencies type

See dotnet/efcore#14698
@ajcvickers ajcvickers modified the milestones: 3.0.0-preview3, 3.0.0 Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. providers-beware type-enhancement
Projects
None yet
Development

No branches or pull requests

1 participant