Proposal: moving towards discoverable config files

Very interesting! I remember you guys discussing the configuration challenge during ElixirConf.EU; I am really happy that you were able to come up with such a clean looking solution! I definitely think that inversion-of-control is the way to go with configuration files. :+1:


I had the same question, especially since it was combined with the example where we have multiple function clauses based on the current building target.

I personally think that if_exists is a lot more explicit in what it checks for than optional.


Question: What would, using the new Application.Config, be the way to read out system environment variables during compilation time? Is the idea to ‘just fall back on the current Mix.Config in that case’ or will that one be deprecated at some point?

Note this is already the case today. We want to improve this distinction in the future. This proposal is the first step towards this path: it allows us to better traverse configs and remove the dependency on Mix.

None of the usages and patterns will change. We are not ultimately changing how we write configs at this point, we are just allowing them to be a bit more discoverable.

1 Like

While I think there’s a lot of good points addressed with the above changes, I do think that keeping single system of configuration files and using it at build and run time will be confusing.

At the moment you have to do some gymnastics to get it working with replace_os_vars and special escaping of strings to support runtime configuration options. It’s not ideal.

I think that inversing the way config files are required would also be beneficial, and precisely for the case of umbrella apps where at the moment you can end up with different config when you start something from app directory than you would get from umbrella root. For the simple umbrellas many people would be happy with just top-level config files, and we could get rid of in-app-directory config files altogether. Single config to rule them all, that’d be good, while not taking away the flexibility to do it your way.

Having said that all above, when we inverse things and have the configuration evaluated by default, there will be a lot of compile-time settings being evaluated at runtime. This is not great because the user cannot immediatelly tell what is what. Let’s have a look at few examples from my config files:

config :logger, :console,
  format: "$time $metadata[$level] $message\n",
  metadata: [:request_id, :application, :module, :ip, :endpoint, :uid, :oid, :type, :custom]

is this a run-time or compile-time setting?

How about Phoenix endpoint like this:

config :ui, UI.Endpoint,
  url: [host: "localhost"],
  render_errors: [view: UI.ErrorView, accepts: ~w(html json)],
  pubsub: [name: UI.PubSub, adapter: Phoenix.PubSub.PG2]

Is this compile-time, or run-time? Can you swap “host” at run-time? Can you change others too? See, this is confusing if this is going to be executed at run time and compile time alike.

At least currently you can, looking at the config files identify which are run time options (because you used escaping/env vars) on these. It’s not ideal but you can figure it out having already existing config file in front of you. Think about that new dev-ops engineer who is taking over maintaining of the project. They’ll know what the settings are by looking at the current config files - something they no longer can do if we implement evaluation of config files at run-time by default.

What I would propose instead

1. Let’s keep the current system but make it slightly more flexible, so you can actually inverse the control yourself.

This means we add config_path as proposed by @josevalim, and it would default to config/config.exs. From there, it can either include other config files (as does now) or you can walk away and inverse the control by not including other configs in that file and manipulating config_path instead. New apps, generators etc. can do it by default at some point.

2. Introduce runtime config

That can be runtime_config_paths and config/runtime_config.exs by default. That can work precisely the same as current Mix.Config, but without relying on Mix to exist, like in the proposal above. To make things consistent, I would even allow these config files to include other config files. In short: Mix.Config would just be kept as name for backward compatability, but the code itself can be moved to Application.Config so we do not maintain 2 code bases.

The runtime config is then clear place where you put your configuration that is going to be evaluated at run time, so you can expect System.get_env and similar to work here.

In dev, when you run iex -S mix it would:

  1. evaluate config/config.exs (i.e. compile-time config)
  2. compile the app
  3. evaluate config/runtime_config.exs (i.e. run-time config)
  4. run the app

while in prod, when started a release executable, where there is no mix only steps 3 & 4 would be performed.

Objective

The proposal above is 100% backward compatible. We can give time for library authors to separate their run-time and compile-time configuration options. Phoenix would generate some compile-time config in config/config.exs, but would move run time configuration to config/runtime_config.exs by default. If you still need to use some old hex package and are forced to use old-style runtime config, you will still be able to do it with this hybrid approach. But with time we get to end up with nice separation of run-time and build-time configuration options, which will make dev-ops engineer’s lifes way easier.

4 Likes

I’m generally in favour of the proposed changes because I think they will simplify some frequent scenarios, most notably fetching settings from OS_ENV.

Another benefit is that config is now present on the server in Elixir syntax, not the Erlang one. Though, in my limited experience I can’t recall a single time when I edited sys.config in prod.

I’ve recently posted a rant on app config which was partly motivated by some discussions I’ve had with @josevalim and @bitwalker, partly by some discussions I’ve had with colleagues, and partly by my continuous distaste for ad-hoc config files (which dates back from my rails days).

It’s a long article, but TL;DR is that while I agree that config scripts are very convenient, and even in the current form flexible, the outcome is that app env tends to be bloated with all sorts of things which are not configuration at all. Even in this thread there have been a couple of examples:

config :code_stats,
  commit_hash: System.cmd("git", ["rev-parse", "--verify", "--short", "HEAD"]) |> elem(0),
  version: Mix.Project.config()[:version]

These are not system configuration parameters, they’re constants derived from the current state of code on the build machine.

config :ui, UI.Endpoint,
  url: [host: "localhost"],
  render_errors: [view: UI.ErrorView, accepts: ~w(html json)],
  pubsub: [name: UI.PubSub, adapter: Phoenix.PubSub.PG2]

As I mentioned in the article, :render_errors and :pubsub are parameters to the library, not configuration.

It could be argued that host is a system configuration, and having it here allows you to change it without needing to redeploy the system. In practice, I don’t think this is a common scenario, and even if I needed to do it, I probably wouldn’t mind redeploying the system. In other words, if you expect that the host will have to be changed and deploying will not be an option, that’s a compelling reason to make host an explicit system property. Otherwise, it’s a case of YAGNI :slight_smile:

Therefore, Instead of further expanding the support for config scripts, say by explicitly separating compile time and runtime scripts, I think that as a community we should first consider how much stuff do we place inside config for mere convenience, and as a consequence how much our config scripts and app envs become bloated with data which is not a part of the system configuration. Perhaps a better way is to promote runtime configuration from the application code (not a config script) as a recommended practice. This can’t handle all scenarios, but I believe that it can handle most of them, and that it’s a better approach.

All that said, the proposed changes look good to me, as they seem fairly limited, but as a benefit they should make hacks such as replace_os_vars obsolete, and they should in general bring OTP releases and local development closer.

10 Likes

The proposal does not change this aspect of configuration and this exact issue exists today. To be clear, I am not saying we shouldn’t discuss it. I want to rather understand the root cause. Are we thinking this proposal makes it worse? Were folks not aware this issue existed? Or do we want to fix this issue now that we are discussing configuration?

Regarding runtime_config.exs, I dislike adding more configuration files because it is just matter of time until it becomes runtime.dev.exs, runtime.prod.secret.exs, etc. However, before we settled on this current proposal, we also discussed introducing a on_boot/1 block to configurations:

use Mix.Config

on_boot do
  config :ui, UI.Endpoint, url: [host: System.get_env("HOST")]
end

...

In this case, only what is inside the on_boot block is evaluated later. The on_boot block would also be restricted to not allow imports and similar. Both mix and releases would only run the on_boot blocks when starting up the project. So as you said, we keep the dynamic aspects that we have today and make only part of the configuration restricted.

However, this proposal (both yours and mine) does not solve the issue of when you are using a dynamic value to a compile time configuration. For example:

on_boot do
  config :ui, MyApp.Endpoint, some_compile_time_config: System.get_env(...)
end

Both runtime_config.exs and on_boot will gladly set this and it will have no effect. This issue also exists with the initial proposal too. In a nutshell, we can’t fully solve this issue without introducing a declarative way to describe configuration, which is something that may be added in the future, but definitely out of scope for now. However, we can consider introducing on_boot instead of the current proposal.

There are different problems to solve and while it can be tempting to solve all of them right now, gradual improvements will allow us to iterate and reap benefits on every release.

1 Like

@sasajuric I think there’s one fundamental thing we don’t agree on when looking at the mix config.

I don’t agree with the premise that mix config is for operator configuration - it is most definitely not. Things that are usually operator configuration - API keys, component addresses should not go in there. But things such as configuring rendering or names of components, etc, should be there. That’s what allows us to have loosely coupled components that are joined through configuration (great example are pubsub name, etc in the phoenix endpoint configuration) - this is a good thing.

I agree with what you said that we violate this for convenience - e.g. we set Ecto database credentials in the config files or phoenix port number. My view, though, is that this violation goes the other way - everything else belongs to mix config files, but things that operators might want to change don’t. They are better served by environment variables or other ways (including JSON files that you mentioned).

@hubertlepicki I think the proposed change wouldn’t “fix” anything in the compile-time vs runtime confusion space. It would be equally as confusing as it is right now. I agree that’s a problem, but it is not a problem this proposal tries to solve. We should have a separate proposal for addressing this problem.

Addressing your proposal for separate runtime and compile-time config files - that’s an idea I had in the past as well. The downside is that if you want to have both per-environment (and you’d probably do want that), now you get 8 config files - the problem of what goes where and how to understand what is the final value just got even worse not better.

6 Likes

I think his proposal does solve one issue though: which is the user intent. We have two intents here: the intent of the user of the library and the intent of the writer of the library.

Ideally we want to conciliate both intents: if the user of the library wants to configure something on boot, then we want to check that with the intent of the writer of the library: can that configuration be set on boot or is it a compile time configuration?

This proposal is not about the library intent but about the user intent. Having on_boot or runtime_config could make the user intent clearer. Although, as you said, it won’t solve the issue of setting a compile-time configuration at runtime. This issue will remain unsolved regardless of the proposals listed here.

I’d be very happy to discuss this further, but I’m worried that it’s going to cause needless noise in this thread. Maybe we should move this to a separate discussion? If so, can someone please do it, because I either lack the permissions or the know-how :slight_smile:

Edit: replied here

1 Like

I’m in favor of all the things in this proposal. I mentioned runtime vs compile time, just because we were talking about config. It’s already an issue today, so it doesn’t have to be solved in this.

This may also be tangentially related, but there are multiple ways to provide configuration besides the ways mentioned here:

  1. Environment variables (solved for)
  2. Secrets file in config format e.g. prod.secrets.exs (solved for)
  3. Individual secrets mounted on the file system. This is common in an environment using docker where a secret is mounted as an in memory disk, which provides better security than an environment variable. (workable, but maybe not ideal)
  4. A secrets manager, like vault. Where config is leased from a manager that has the ability to invalidate secrets. (not solved for)

This could be moved to another discussion, but I would like the ability to specify another type of configuration provider that may be more suitable for the type of configuration that is being read.

What are your thoughts on the proposed on_boot? /cc @sasajuric @hubertlepicki

Yup, definitely a separate discussion. If we had configuration providers, this is about improving the built-in provider. Custom providers is a separate topic and I believe @bitwalker was working on this.

This would make me personally happy.

Does this mean that on_boot would replace use Application.Config?

@sasajuric theoretically there would be no need for Application.Config, as outlined in this proposal, but I still think we need to have Application.Config in case anyone wants to customize a release after it is assembled (and we can’t depend on Mix by then as well). In other words, I would still introduce Application.Config but it is for another purpose rather than file discovery (it could even be a separate proposal).

@josevalim Right, I’m just trying to figure out what’s the choice here. So if I get it right, you’re now asking us what do we think about use Application.Config vs on_boot, where the latter would be used to explicitly denote the parts which are evaluated at runtime?

I.e. with on_boot, it would look like this:

# we're using mix config, as before
use Mix.Config

on_boot do
  # evaluated at runtime
  config :ui, UI.Endpoint, url: [host: System.get_env("HOST")]
end

# evaluated at compile time
config :foo, :bar, System.get_env("BAZ")

Is that the choice?

I’m a fan of the on_boot/1 proposal.

1 Like

Almost. Let’s forget about Application.Config completely. The choices are:

  1. Introduce on_boot as an explicit block of code to be evaluated on boot (e.g. during a release or during mix app.start)

OR

  1. Restrict import_config to be static we can copy configuration files as is to a release and have the same configuration files be executed at both compile time and runtime without a distinction of “when” each is happening

Ah, that sounds neat, and it seems like it’s a smaller change from the perspective of user devs. In addition, as others have mentioned it, it makes the execution context explicit, which seems like a better choice.

Why did you settle on the current proposal then?

1 Like

The current proposal was seen a smaller step. We could always add on_boot later and we thought restricting import_config was going to be a good thing nonetheless. But I guess the opposite direction is also true: we can add on_boot now and restrict import_config later.

3 Likes

on_boot/1 sounds good :+1:

One question though, from the initial proposal I had a feeling that it’s a “hack” or a “workaround” that would temporary make stuff clearer without fixing anything, but reading the discussion I get a feeling that it’s a first step towards unifying the configuration and the workings of mix and releases, is that so?

There was an article describing the general direction Elixir is moving: https://dockyard.com/blog/2018/02/28/elixir-deployment-tools-update-february-2018

To remove the awkward transition between development and production, we really need our development tooling to be built on releases under the covers. In short, if you run iex -S mix or mix run, these should effectively be the equivalents of bin/myapp console or bin/myapp foreground. If you are always running releases, then there is no transition to be made between development and production.

I very much like the idea, am I right to assume that it’s still the end goal?

2 Likes

i think the core problem with elixir’s use of an executable script to populate the application env and the use of Application.get_env/3 as the means to retrieve config is that it makes it acceptable and idiomatic to put configuration in config.exs. this is fine for examples and most standalone apps but it breaks down for users who have to integrate elixir into existing ecosystems where things like vault/etcd/dynamodb/encrypted json blobs/etc are used. at two different jobs i’ve had to fork phoenix, ecto and other applications to break their dependence on an evaluated config.exs to allow for use within the environment that was already established. at my latest job i’m just not using elixir because the burden to introduce it was too high

i don’t think there’s anything wrong with either of these proposals but i think incremental improvements to config are not enough. the risk is that it gets good enough that no further work is done and the real issue (that of libraries not exposing their internals in a way that is flexible enough for more than just archetypal users) is never addressed

5 Likes