I was refering to the current style, which is not flexible enough. At my company we’re fetching some part of configuration from a json file. Previously, we did it from etcd. So the way Phoenix works was not good enough for us, and we had to do some shenanigans during app startup, and update app env to make it work. The same thing holds for Ecto. In other words, in all of our Elixir apps we’ve already had a couple of cases where app env was not a good approach for us. So that definitely sucks, and this is why I think taking options through parameters is more flexible. Because with that I can fetch values in arbitrary ways.
Admittedly, the new proposal amends this issue for Phoenix. But it does it in what I find a strange and a confusing way. With the new proposal, when reading the code, I’ll see something like:
def load_from_system_env(opts) do
port = System.get_env("PORT") || raise "expected the PORT environment variable to be set"
{:ok, Keyword.put(opts, :http, [:inet6, port: port])}
end
The issue I take with this is that when I read this code, it’s completely unclear when (and if at all) is it invoked. I need to know particularities of Phoenix, and need to be aware of configuration for each specific env. I find this confusing. I’m also worried that other libs will start to use this pattern.
I think I understand your concerns about code reload, but the points I argued are:
- It is my impression that most people are not doing it.
- It can still be done for many cases when options are passed as args.
- Even for endpoint reconfiguration it can be done, though it does require restarting of the entire endpoint (but not the rest of the system).
So this leaves us with the solution that is optimized for the case of “I want to reconfigure the endpoint with restarting as few things in it as possible”. I’m not going to say this is not a valid case, but I haven’t seen it in practice myself. In contrast, I’ve seen multiple cases where app config is making Phoenix (and Ecto) usage more complex than it could be.
Now, if the callback function was always invoked, I’d like it better, because then Phoenix becomes more like a behaviour, so it’s consistent to the other existing abstractions.
You could consider decoupling the callback module from the behaviour module. I actually dislike the fact that the Endpoint conflates the plug module and the process behaviour.
As I explained I had cases when this doesn’t suffice, and then I needed to resort to some trickery. This is also going to be the case for dynamic endpoints, because I have to devise unique key names, and manually delete them when I’m taking the endpoint down.
The explicit Application.fetch_env in the client code is not much more typing, it can be created by the generator. It is more flexible because you can replace it, or combine it with something else. It still promotes multiple environments. And it reduces the magic, because it’s now clear to a reader where do the settings come from (currently you need to know that Phoenix will read the setting from the key which has the same name as the endpoint module).
I already tackled multiple envs (explicit Application.fetch_env call inserted by the generator). My first take on compile time configs would be to question if they are really needed. If we can turn them into runtime settings it would be great. If not, then this remains an open question.
Does this mean that my GenServers shouldn’t accept options as parameters anymore? Should everybody write a more complex code (and configuring through init is IMO more complex), just because some day they might want to do code reloading?
Note that I’m not saying that Phoenix (or other libs) shouldn’t support proper code reloading. I do however feel that there’s a big difference between saying “If you want this, you need to do that” and “You should always do it like this because you might need it someday”. This is IMO especially relevant in the context of code reloading, which is itself difficult to get right, and should be reserved for very special cases.
Let’s say that I have something like:
