deferred_config icon indicating copy to clipboard operation
deferred_config copied to clipboard

Unable to use deferred_config with mix tasks

Open aforward opened this issue 7 years ago • 2 comments

I was looking for some guidance on how to properly use deferred_config when running mix tasks. In particular this is for running ecto tasks.

First, here is my test config.

use Mix.Config

config :myapp, Myapp.Repo, [
  adapter: Ecto.Adapters.Postgres,
  database: "myapp_#{Mix.env}",
  username: {:system, "DB_USERNAME"},
  password: "",
  hostname: "localhost",
]

When I run iex -S mix, I properly see the env variable substituted.

iex(1)> Application.get_all_env :myapp
[{Myapp.Repo,
  [adapter: Ecto.Adapters.Postgres, database: "myapp_dev",
   username: "postgres", password: "", hostname: "localhost"]},
 {:ecto_repos, [Myapp.Repo]}, {:included_applications, []}]

But if I run mix ecto.create (for example), then the variable is not, as the application is not started, so the deferred injection isn't run.

$ mix ecto.create
Compiling 1 file (.ex)

07:19:22.351 [error] GenServer #PID<0.264.0> terminating
** (ArgumentError) argument error
    :erlang.iolist_size([<<0, 3, 0, 0>>, [[[], "user", 0, {:system, "DB_USERNAME"}, 0], "database", 0, "postgres", 0], 0])
    (postgrex) lib/postgrex/messages.ex:221: Postgrex.Messages.encode_msg/1
    (postgrex) lib/postgrex/protocol.ex:2018: Postgrex.Protocol.msg_send/3
    (postgrex) lib/postgrex/protocol.ex:551: Postgrex.Protocol.startup/2
    (postgrex) lib/postgrex/protocol.ex:475: Postgrex.Protocol.handshake/2
    (db_connection) lib/db_connection/connection.ex:134: DBConnection.Connection.connect/2
    (connection) lib/connection.ex:622: Connection.enter_connect/5
    (stdlib) proc_lib.erl:247: :proc_lib.init_p_do_apply/3
Last message: nil
State: Postgrex.Protocol
** (Mix) The database for Myapp.Repo couldn't be created: an exception was raised:
    ** (ArgumentError) argument error
        :erlang.iolist_size([<<0, 3, 0, 0>>, [[[], "user", 0, {:system, "DB_USERNAME"}, 0], "database", 0, "postgres", 0], 0])
        (postgrex) lib/postgrex/messages.ex:221: Postgrex.Messages.encode_msg/1
        (postgrex) lib/postgrex/protocol.ex:2018: Postgrex.Protocol.msg_send/3
        (postgrex) lib/postgrex/protocol.ex:551: Postgrex.Protocol.startup/2
        (postgrex) lib/postgrex/protocol.ex:475: Postgrex.Protocol.handshake/2
        (db_connection) lib/db_connection/connection.ex:134: DBConnection.Connection.connect/2
        (connection) lib/connection.ex:622: Connection.enter_connect/5
        (stdlib) proc_lib.erl:247: :proc_lib.init_p_do_apply/3

To work around this, I leveraged the init/2 function from Ecto

defmodule Myapp.Repo do
  use Ecto.Repo, otp_app: :myapp

  def init(_, config) do
    config
    |> DeferredConfig.transform_cfg
    |> (fn updated -> {:ok, updated} end)
  end

end

But I was wondering if there was a better way.

aforward avatar Aug 23 '17 11:08 aforward

(Obligatory 'what version of Ecto?')

Another option, either instead of or in addition to the Ecto-specific init/2 callback, is supporting the running of Ecto tasks like pending migrations/repo creation at app run time, either automatically on startup, or as something that can be triggered manually. That may be something you need to do some day anyway, as running Mix tasks programmatically plays nice with releases.

Regarding the init/2 callback and better ways:

That should work, and it preserves nice 'dev UX'. It always does feel a little weird to populate configs 'outside of' the actual app. It's config that conceptually 'belongs to' your app, but Ecto is one of those libraries where, when you use it in your app, you're really starting up other apps as well, and then starting up servers under your own app that communicate with them. Configuration in this scenario calls for nuance when interpreting the advice in Elixir.Application

Keep in mind that each application is responsible for its environment. Do not use the functions in this module for directly accessing or modifying the environment of other applications (as it may lead to inconsistent data in the application environment).

Of course, mix is kinda-sorta an 'other application'. :)

So when we run an Ecto mix task, it'll try to read config from Myapp, but it doesn't assume it can safely start it up because it's a dependency that doesn't really know that much about your app, despite an Ecto.Repo being "a part of" your app from a supervision standpoint, and so it has to hope that the config is usable as-is. That's why Ecto benefits from the recent init callback, and why making use of it the way you are makes sense if you want to run CLI mix tasks.

It seems at first like a mild annoyance that this can't be supported automatically, and to need to say "and for libraries that depend on your app's config outside of your app's OTP lifecycle, like in some Mix tasks, see if they give you an init callback and run DeferredConfig.transform_cfg there as well." Which I should add to the docs.

But the only way to avoid it would be if there was a strong convention for config handling at the OTP app level, a kind of magical MyApp.get_runtime_env callback that all apps had, so that if a library meant to run supervised services under an app provides an additional value proposition that

  1. doesn't depend on the app running, but
  2. does depend on parent app config

(as is the case here), they could "ask their parent" for populated config without needing that parent to be started. If that were the case and widely used -- maybe in the :init Application.start_phase -- then in theory this could be handled automatically by a library like Ecto. However, in practice -- what if the 'initialization' of the app's config depends on a service in the supervision tree running, or on a response from another app? Which is why I said this would have to be a magical callback ;)

So Ecto is right to use init/2, and so should other libraries in the same boat -- but it should be pretty rare to have things that rely on an app's env/mix config, but which explicitly do so outside of that app's OTP lifecycle. Once things get complex or domain specific enough, it makes sense to move inside the lifecycle, to run time.

mrluc avatar Aug 23 '17 17:08 mrluc

See also the interesting

  • https://github.com/elixir-lang/elixir/issues/6016#issuecomment-295804849,
  • which led to https://github.com/elixir-ecto/ecto/issues/2038
  • and then the commit https://github.com/elixir-ecto/ecto/commit/88665d426a3bcecb3935f8d57a0c14fd08539df7 and discussion.

mrluc avatar Aug 23 '17 19:08 mrluc