josevalim

josevalim OP

Creator of Elixir

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
Post #12
hubertlepicki

hubertlepicki

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
Post #13
sasajuric

sasajuric

Author of Elixir In Action

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
Post #14
josevalim

josevalim OP

Creator of Elixir

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
Post #15
michalmuskala

michalmuskala

@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
Post #16
josevalim

josevalim OP

Creator of Elixir

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.

0
Post #17
sasajuric

sasajuric

Author of Elixir In Action

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
Post #18
blatyo

blatyo

Conduit Core Team

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.

0
Post #20
josevalim

josevalim OP

Creator of Elixir

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.

0
Post #21
hubertlepicki

hubertlepicki

This would make me personally happy.

0
Post #22