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

Discussion: Configuration #34

Closed
chrismccord opened this issue Feb 10, 2014 · 18 comments
Closed

Discussion: Configuration #34

chrismccord opened this issue Feb 10, 2014 · 18 comments

Comments

@chrismccord
Copy link
Member

Handling configuration is up for discussion. Currently, the only portion of the application that needs configuration support is the Router, but we need to think ahead on how we'd like to handle Phoenix configuration in general. I'd like to frame configuration within the context of a non-monolith setup. We have a couple options off the top of my head:

  1. Simple code based configuration. We could let other modules (i.e. Routers), specify a config module. i.e:
defmodule Router do
  use Phoenix.Router, port: 4000

  def conf, do: Conf

  resources "users", Users
  ...
end

defmodule Conf do
  def domain(:prod), do: "example.com"
  def domain(:dev),  do: "example.dev"

  def enforce_ssl?(:prod), do: true
  def enforce_ssl?(_),     do: false
end
  1. YAML/JSON configuration
    I'm not sure what existing tooling is available within Elixir/Erlang for such support. I'm a heavy user of YAML, and it typically works well, but my current thoughts are it's best to limit the number of serialization formats we use, and I'd prefer staying as close to pure code where possible. For example, we're obviously going to have JSON support in Phoenix, so if we go the serialized config route, maybe it makes sense to just stick a single format.

This is just a brain dump of ideas. I'd love to hear what everyone thinks and other solutions not mentioned. My current preference is code based configuration, but this rules out nesting, unless we write a Config lib that sprinkles in some macro facilities.

@slogsdon
Copy link
Contributor

If you go with Option 1, why not have a config/1 function on your Conf module that returns a Keyword? Something like:

defmodule Conf do
  def config(:prod) do
    [
      domain: "example.com",
      enforce_ssl: true
    ]
  end

  def config(_) do
    [
      domain: "example.dev",
      enforce_ssl: false
    ]
  end
end

which would prevent having to pass the environment every time a configuration value is needed. A macro could possibly clean the definition up as well while compiling to something similar.

@josevalim
Copy link
Member

A couple things: putting it in the router module seems to be beyond routing concern. Putting it in another module seems like a good idea but don't hardcode it. You will need a mechanism to specify which Conf module you want to use.

Another idea is that Mix will come with its own configuration mechanism. However, it may take a long while as we are waiting on a pull request sent to Erlang to be accepted.

@chrismccord
Copy link
Member Author

@slogsdon The keyword list approach could work nicely and reduce a lot of repetition of env. Thanks for the suggestion.

@josevalim Per my first example, do you think specifying the conf/0 from the Router is beyond its concern, or is that the kind of mechanism you're advocating? Do we have a working spec on what mix configuration will look like? It might be nice for us to follow closely to what mix eventually provides.

This is also where conventions make a lot of sense. When we mix phoenix.new, the default Router is generated under the MyApp.Config.Router namespace. We could be smart and look for a config module based on the current Application prefix of the module(s) we're operating in, and have a fallback mechanism to specific which Conf module is to be used. We could also have Router optionally set an attribute for which "application" they are a part of, and conventionally check that application namespace for a Config module.

@josevalim
Copy link
Member

@josevalim Per my first example, do you think specifying the conf/0 from the Router is beyond its concern, or is that the kind of mechanism you're advocating?

I am not sure. :)

Do we have a working spec on what mix configuration will look like? It might be nice for us to follow closely to what mix eventually provides.

None yet, sorry. As we are still waiting on some definitions coming from Erlang-land.

This is also where conventions make a lot of sense. When we mix phoenix.new, the default Router is generated under the MyApp.Config.Router namespace.

Agreed! Just one note: I don't believe a router is configuration. For me router is code. It is one the things I tried to change in Rails but never could (it is even reloaded in Rails!).

@chrismccord
Copy link
Member Author

@josevalim Coming from Rails, I never thought about router living in config/ being unusual, but I agree it's not configuration and I would welcome the default router simply being MyApp.Router. This would even let cleaner application interop, i.e. redirect MyOtherApp.Router.root_url.

Here's an idea for simple conventional configuration. The keyword list approach by @slogsdon might prove to be cleaner, but I'm interested in exploring a function based approach since config dispatch just becomes a function call. It can also allow the user to implement whatever business logic they want around configuration easily. If this turns out to be a terrible idea, the keyword list approach is tough to beat as far as simplicity goes.

defmodule MyApp.Config.Dev do
  use Phoenix.Config.Dev        # defaults

  def domain, do: "example.dev"
  def code_reloading?, do: true

  # custom
  def twitter_api_secret, do: System.get_env("TWITTER_API_SECRET")
end

defmodule MyApp.Config.Prod do
  use Phoenix.Config.Prod       # defaults

  def domain, do: "example.com"
  def enforce_ssl?, do: true
  def code_reloading?, do: false

  # custom
  def twitter_api_secret, do: System.get_env("TWITTER_API_SECRET")
end

defmodule MyApp.Router do
  resources "users", Controller.Users
end

iex> Mix.env
:prod
iex> MyApp.Config.env.domain
"example.com"
iex> MyApp.Router.users_url
"https://example.com/users"

@josevalim
Copy link
Member

I am actually 👎 on the function approach. Treating config as keywords means we are treating config as data and we already have a bunch of functions that know how to work with data. For example, you could easily have:

def config do
  merge regular_config,
    config_from_production_only_file
end

@josevalim
Copy link
Member

Btw, Dynamo had the following approach:

config :server, port: 3030
config :dynamo, templates_path: "..."

What was good in this approach:

  1. Configurations are named and read nicely
  2. Configurations were processed at compilation time

What was bad in this approach:

  1. There was no mechanism for defining configuration at runtime. So you couldn't do:
config :server, port: System.get_env("PORT")

since System.get_env("PORT") would be processed at runtime, just on compilation time. I think the cons is a big one here, I definitely prefer the runtime flexibility than the potential compile time performance gain.

@chrismccord
Copy link
Member Author

What if Dynamo's approach compiled to a function definition?

config :server, port: System.get_env("PORT")
# --->
def server do
  [port: System.get_env("PORT")]
end

iex> Conf.server[:port]
1234

But if we go that far, same dynamo API, but runtime dispatch, why not write straight functions at that point? :trollface: . I do agree we don't want to mix too much behavior with data as far as config goes.

@josevalim
Copy link
Member

Your suggestion of mixing functions with keywords is a good one. 👍

@josevalim
Copy link
Member

However, keep in mind how you would like to integrate that with environments. Merging keywords is easy... merging functions would likely need to be done at runtime (and then possibly slow).

@chrismccord
Copy link
Member Author

We can probably get away with merging keywords at compile time with the above approach. Pluck out the options provided, merged in the defaults at the compile time to produce the final keyword list. For environment handling, I still think using separate modules would be easiest, then looking up the config module from a env/0 function.

ie.

defmodule MyApp.Config.Dev do
  use Phoenix.Config.Dev        # defaults

  config :router, url: "example.dev", ssl: false
  config :server, code_reloading: true
  config :twitter, secret: System.get_env("TWITTER_API_SECRET")
                  key: System.get_env("TWITTER_API_KEY")
end


iex> Mix.env
:dev
iex> MyApp.Config.env.router[:domain]
"example.dev"

It seems like this might hit a sweet spot between performance, runtime dispatch, and keeping configuration as data, so long as we reconcile defaults at compile time.

@josevalim
Copy link
Member

I think the config approach works well if executed at compilation time. For me it feels weird if config :twitter, secret: ... is not evaluate in the module body. For example:

config :twitter, LoadConfiguration.from_elsewhere

How you are going to merge that at compilation time? That said, all approaches are going to have trade-offs. For example, your suggestion of mixing functions and configs is a good one:

def server do
  ...
end

And you could use a cache to handle the merged environment for you, calculated the first time it is requested. For example, MyApp.Config.read(:server) would load the defaults, the environment one and cache them somewhere (likely ETS). This pushes complexity to phoenix though.

I am sorry if I am posing too many problems. :P I have just evaluated those things some time ago and the pros and cons are resurfacing as we discuss different solutions. :)

@chrismccord
Copy link
Member Author

@josevalim Your experience is extremely helpful here and you hold insights that many of us are still discovering, so don't hesitate with your feedback :)

Good point with the LoadConfiguration.from_elsewhere. It could be a surprise that we are both turning config calls to runtime dispatch, but not treating them as runtime behavior for defaults during compile time. I think the ETS approach is one way to go to keep all cases satisfied. Config v1 could be just keyword list based, and we could later expand the internal implementation to use ETS cache, while keeping the same api in user-land.

@chrismccord
Copy link
Member Author

I just shipped the first version of the latest proposal as its own project under the phoenixframework account since this functionality is universal. I'll copy the description here, but please review the full readme and let me how things look.
https://github.com/phoenixframework/ex_conf

Features:

  • Configuration definitions are evaluated at runtime, (but defaulted/merged at compile time), allowing runtime dependent lookups. (i.e. System.get_env)
  • Configuration modules can extend other configurations for overrides and defaults
  • Evironment based lookup for settings based on current Mix.env

For Phoenix, we will by convention place configuration in lib/config/ with a configuration file/module per Mix.env. This will leave us with something like:

# config/app.ex
defmodule MyApp.Config do
  use Phoenix.Config.App

  config :router, ssl: true
  config :twitter, api_token: System.get_env("API_TOKEN")
end

# config/dev.ex
defmodule MyApp.Config.Dev do
  use MyApp.Config

  config :router, ssl: false
  config :twitter, api_token: "ABC"
end

@slogsdon
Copy link
Contributor

I like this. Clean, simple, and compiled. Great work @chrismccord!

@darkofabijan
Copy link
Contributor

Same here. Very nice!

@pma
Copy link
Contributor

pma commented Mar 22, 2014

For configuration purposes, should the environment be set at compile time, instead of being determined at runtime from the MIX_ENV env var?

My point is that in those cases where we may generate different code for prod and dev based on configuration options, if then at runtime the Config module returns values based on MIX_ENV (with the default being dev) we end up with a mismatch between the running code and the config values.

As an example, suppose we changed before_compile in Phoenix.Router to only plug CodeReloader if Config.plugs[:code_reload] == true (at compile time). It would eliminate the call to reload! that returns :noop in Phoenix.Plugs.CodeReloader when running in production. But then, for consistency, if at runtime we read Config.plugs[:code_reload] it should return false, and not true just because MIX_ENV is not defined. We could be running it as part of a release which in production will most likely not bundle mix.

@chrismccord
Copy link
Member Author

@pma Setting at compile time is probably the best approach. I'll need to see what would be involved to make it happen. I'm not sure if I'll need to ensure_compiled on the Config modules, but it's something that I think we can address. I agree that it would be ideal to maintain parity with each environment build instead of runtime env lookup.

Gazler pushed a commit to Gazler/phoenix that referenced this issue Sep 17, 2015
…es-to-reflect-desired-order

renaming files to reflect proper order and content
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants