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

Shared physical file provider with base directory as root #63538

Conversation

mapogolions
Copy link
Contributor

No description provided.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Jan 8, 2022
@ghost
Copy link

ghost commented Jan 8, 2022

Tagging subscribers to this area: @dotnet/area-extensions-configuration
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author: mapogolions
Assignees: -
Labels:

area-Extensions-Configuration

Milestone: -


return new PhysicalFileProvider(AppContext.BaseDirectory ?? string.Empty);
var fileProvider = new PhysicalFileProvider(AppContext.BaseDirectory ?? string.Empty);
builder.Properties[FileProviderKey] = fileProvider;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not certain this is a valid change. Thinking about #63540, the PhysicalFileProvider is IDisposable. Sticking it in the builder.Properties means people may be able to get at it after it has been disposed.

Can you add a reasoning/scenario (maybe in an issue?) for why this change is necessary?

Copy link
Contributor Author

@mapogolions mapogolions Jan 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand from code, the Properties field of the IConfigurationBuilder instance is used like a temporary bag to store some shared data that can be used during construction the IConfiguration . Sticking the PhysicalFileProvider in the Properties of the ConfigurationBuilder also happens in case when we use another extension method the SetBasePath. Could you please explain to me what the difference between these cases? I just thought it would be great to create and use only one PhysicalFileProvider
At the moment, each configuration source calls EnsureDefaults and create own file provider with base directory as root. If we create shared physical file provider then lines of code below will have the same semantic

var builder = new ConfigurationBuilder().AddJson().AddIni().AddAnotherFile()...Build()
var builder = new ConfigurationBuilder().SetBasePath(AppContext.BaseDirectory).AddJson().AddInit().AddAnotherFIle()...Build()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please explain to me what the difference between these cases?

The difference is that calling SetBasePath is an explicit action to stick the file provider into the Properties collection. You can use a single IConfigurationBuilder to build multiple configurations.

If someone disposed the PhysicalFileProvider while it was being held on to the ICollectionBuilder, you can get an ObjectDisposedException if you tried using the ICollectionBuilder again. It should be an explicit action to cache the file provider into the ICollectionBuilder.

@ghost ghost locked as resolved and limited conversation to collaborators Feb 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Extensions-Configuration community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants