elixir-toniq / vapor

Runtime configuration system for Elixir
MIT License
596 stars 37 forks source link

Consider `use Vapor.Application` maybe? #64

Open devonestes opened 4 years ago

devonestes commented 4 years ago

Hey, so I love Vapor's new API, and I'm super happy you've taken the time to put all of this together and continue to refine it - thanks for that!

I'm like 90% sure that what I'm proposing is a bad idea, but I was working yesterday with someone who was new-ish to Elixir (and totally new to Vapor), and they said this:

I think this particular example is interesting because it's configuration inside a function, not a process and so Vapor.load!/1 wouldn't be called once at boot, but every time we call the function (perhaps a concern for hot code paths). Meaning if the application was configured incorrectly, we wouldn't find out until we called the function.

The code they were talking about was something like this:

def sign(message) do
  key = Application.get_env(:my_app, :private_key)
  do_sign(message, key)
end

And they were under the impression that to use Vapor they'd need to do something like:

def sign(message) do
  %{key: key} = Vapor.load!([%Env{bindings: [key: "PRIVATE_KEY"]}])
  do_sign(message, key)
end

Personally I love having the API of Vapor be just a function call, but I think this is leaving some folks with confusion about when and how to use it. I might be missing something here, but I see this as basically only being used in an application's start/2 callback, and so maybe the design of the API could make that more explicit by hiding the underlying function that it calls?

I'm thinking about something like this:

defmodule VaporExample.Application do
  use Vapor.Application,
      env: %{bindings: [db_url: "DB_URL", db_name: "DB_NAME"]},
      file: %{path: "config.toml", bindings: [kafka_brokers: "kafka.brokers"]},
      file: %{path: "config.json", bindings: [port: "port"]}

  def start(_type, _args, config) do
    children = [
       {VaporExampleWeb.Endpoint, port: config.port}
       {VaporExample.Repo, [db_url: config.db_url, db_name: config.db_name]},
       {VaporExample.Kafka, brokers: config.kafka_brokers},
    ]

    opts = [strategy: :one_for_one, name: VaporExample.Supervisor]
    Supervisor.start_link(children, opts)
  end
end

Which would translate to something like:

defmodule Vapor.Application do
  defmacro __using__(providers) do
    quote do
      use Application

      def start(type, args) do
        config = Vapor.load!(unquote(providers))
        start(type, args, config)
      end
    end
  end
end

This is just a quick and silly idea, and maybe some additional documentation could help here (or something like a set of "here's how you do these common things with Vapor" examples, or a wiki or something), but this was just on idea that jumped to mind, and I thought that seeing this feedback from someone a bit less experienced with Elixir might be helpful in making an API that even those who don't yet fully understand the Elixir boot and config process can use successfully.

keathley commented 4 years ago

I definitely think this is an issue. It's partially a documentation problem and some examples of real-world usage would go a long way. I've been considering a more convenient API for building up configuration, but it was largely inspired by the existing mix config syntax. Something like this:

defmodule MyApp.Config do
  use Vapor.ConfigBuilder

  config :kafka, [
    env: [
      brokers: "KAFKA_BROKERS",
    ]
  ]

  config :database, [
    env: [
      {:port, "DB_PORT", map: &String.to_integer/1},
      {:host, "DB_HOST"},
    ]
  ]
end

Now that Group is built into Vapor, it would be straightforward to build up the "configuration tree". This module could provide .load, .get, and .set functions that could be used to access configuration values. The values themselves could be stored in something like ets or persistent term.

I find the idea of a Vapor.Application module intriguing. The thought hadn't occurred to me. But its actually a formalization of exactly how we use Vapor at work. We typically define a "configuration" function that declares everything we want. The first function call in start/2 is config = Vapor.load!(configuration()). From that point, it would still be up to the developer to put the configuration into the application env or persistent term or whatnot.

I need to think through all of the implications. But in the meantime, I'll see what I can do about improving the documentation.