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

Configuration constructor requires full path if relative paths are used to add Configuration files #13

Open
davidfowl opened this issue May 3, 2015 · 14 comments

Comments

@davidfowl
Copy link
Member

Old code:

public Startup(IHostingEnvironment env)
{
      // Setup configuration sources.
      Configuration = new Configuration()
         .AddJsonFile("config.json")
         .AddEnvironmentVariables();
}

This will throw the following exception now:

System.InvalidOperationException: Unable to resolve path 'config.json'; construct this IConfigurationSourceRoot with a non-null BasePath.

New code:

public Startup(IHostingEnvironment env, IApplicationEnvironment appEnv)
{
      // Setup configuration sources.
      Configuration = new Configuration(appEnv.ApplicationBasePath)
         .AddJsonFile("config.json")
         .AddEnvironmentVariables();
}
@adamralph
Copy link

Are you keeping the parameterless constructor?

@bricelam
Copy link

bricelam commented May 4, 2015

Why isn't this done "by convention"? It seems like the sensible default.
Is it for the same reason that AppContext.BaseDirectory isn't equal to IApplicationEnvironment.ApplicationBasePath?

@dougbu
Copy link
Member

dougbu commented May 4, 2015

@adamralph the parameterless constructor remains because not all configurations have anything to do with files on disk. Also fully-qualified paths passed to AddJsonFile() (and so on) work fine without a base path passed to the constructor.

@bricelam "convention" was based on information from DI that is not available in the Configuration constructor. Used to rely on the (hideous) static IServiceProvider field.

@bricelam
Copy link

bricelam commented May 4, 2015

@dougbu So yes, if AppContext.BaseDirectory, the correct/non-hideous way of getting the app's base path, returned the right value it could be used instead of the static service provider field. I hit the same issue in the SQLite ADO.NET provider.

We should revert both of these when aspnet/dnx#1253 is resolved.

@dougbu
Copy link
Member

dougbu commented May 4, 2015

@bricelam no, aspnet/dnx#1253 is unrelated since the application base is not the same as the project base needed to find individual configuration files. One example scenario is MVC functional tests which load numerous web sites, many with their own configuration files, into a single application. But the issue is more general...

@davidfowl
Copy link
Member Author

The intent is to remove CallContextServiceLocator from as many APIs as we can because it makes them dnx specific.

@adamralph
Copy link

When constructing with no path, why can't the current directory be used for relative paths?

@dougbu
Copy link
Member

dougbu commented May 5, 2015

@adamralph what "current directory"? The user's $pwd when they invoked dnx? Pretty much everything else needs a service to determine, exactly what this change avoids.

@adamralph
Copy link

So I guess dnx has no static Directory.GetCurrentDirectory().

If so, that's perfectly understandable. I can understand the motivation to omit it.

@radenkozec
Copy link

This is very ugly. We need to pass path where config file is located and all the paths are usually stored in config itself.
I now need to refactor 54 methods in project to solve this issue.

@davidfowl
Copy link
Member Author

Why are you newing up the config in 54 places?

@radenkozec
Copy link

I have a class library project which is referenced from ASP.NET project.
I had there this piece of code which is not nice but...

 public static string GetConnectionString()
 {
                if (string.IsNullOrEmpty(connectionString))
                {
                    var builder = new ConfigurationBuilder()
                        .AddJsonFile("config.json");
                    Configuration = builder.Build();
                connectionString = Configuration.Get("Data:DefaultConnection:ConnectionString");
                }
            }
            return connectionString;
 }

This method is called 54 times from different places.
Here I need to inject IApplicationEnvironment into constructor of this static class into project which is not ASP.NET so I need to add reference to Microsoft.Framework.Runtime.Abstractions
and things get complicated.

In the end I have managed to solve my issue by passing static string variable into this class from startup of my ASP.NET 5 project.

@roydukkey
Copy link

@davidfowl It seems that the new code you suggest is no longer current. Is that right?

Application startup exception: System.InvalidOperationException: Unable to resolve service for type 'Microsoft.Framework.Runtime.IApplicationEnvironment' while attempting to activate 'Web.Startup'.
   at Microsoft.Framework.DependencyInjection.ActivatorUtilities.ConstructorMatcher.CreateInstance(IServiceProvider provider)
   at Microsoft.Framework.DependencyInjection.ActivatorUtilities.CreateInstance(
IServiceProvider provider, Type instanceType, Object[] parameters)
   at Microsoft.Framework.DependencyInjection.ActivatorUtilities.GetServiceOrCreateInstance(IServiceProvider provider, Type type)
   at Microsoft.AspNet.Hosting.Startup.StartupLoader.LoadMethods(Type startupType, IList`1 diagnosticMessages)
   at Microsoft.AspNet.Hosting.Internal.HostingEngine.EnsureStartup()
   at Microsoft.AspNet.Hosting.Internal.HostingEngine.EnsureApplicationServices(
)
   at Microsoft.AspNet.Hosting.Internal.HostingEngine.BuildApplication()
Hosting environment: Development
Now listening on: http://localhost:4689
Application started. Press Ctrl+C to shut down.

@aspnet aspnet locked and limited conversation to collaborators Nov 16, 2015
@Tratcher
Copy link
Member

@roydukkey IApplicationEnvironment is no longer required. See #88.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants