This is a very well prepared proposal. I have one question and one further
proposal to improve it.
By removing the application callbacks we remove the primary place of injecting
dynamic configuration into an application. Dynamic configuration is already a
major hurdle, without this proposal or with it, but I'm afraid this will only
worsen the situation. It's difficult (and inconsistent in releases vs running
with mix) to configure applications with dynamic values - be it from
environment variables or external services like zookeeper. Is there a planned
proposal to improve upon this (and I should stop thinking about it) or are the
application callbacks still the best place for this? If so, how do we make it
more obvious?
This is mostly an open question and I don't have a good answer for today, but
I'd like to know where we stand.
Now let's see, how I think the proposal could be improved.
How would an example GenServer callback module will look like (without the
proposed "automatic child_spec" and with the new GenServer proposal [1]):
defmodule MyServer do
@behaviour GenServer
def child_spec(_args) do
%{id: MyServer, start: {__MODULE__, :start_link, []}, restart: :transient}
end
def start_link(args) do
GenServer.start_link(__MODULE__, args, args)
end
def init(args) do
{:ok, new_state(args)}
end
# ...
end
I can see at least three issues here:
* the __MODULE__ part feels repetitive - there should be a way to specify it
only in one place.
* we need to know if the behaviour should be a worker or a supervisor child.
* in practice it might be hard to distinguish what are the arguments to the
callback itself, and what are the arguments to the behaviour (e.g. name). Now I
need to understand the flow of the arguments through 3 functions, instead of
two. Where before there was only start_link and init (and it was sometimes
already confusing), now we have child_spec, start_link and init - even more
confusion awaits.
The "automatic child_spec" part of the proposal addresses the first and second
issues, but unfortunately it introduces a lot of complexity and additional
macro "magic". With the current work in GenServer to migrate away from
overridable functions, I'm rather surprised by this. Additionally this does not
address the third, even more important issue. Hiding things behind macros is
not reducing the complexity - even worse it is hiding it. It makes defining the
module "simple", but unfortunately not "easy"[2].
I propose we embrace the child_spec fully and use it to replace start_link
entirely. Instead of providing a raw map in child_spec, we would call a
child_spec function in the parent behaviour, delegating the responsibility of
knowing if this should be a "supervisor" or "worker" similar to the "automatic
child_spec" proposal, which was one of the main issues it embarked to solve.
Additionally we could define the child spec in a way that does not require the
start_link function.
How would a module implemented with this look like?
defmodule MyServer do
@behaviour GenServer
def child_spec(_args) do
GenServer.child_spec(__MODULE__, args)
end
def init(args) do
{:ok, new_state(args)}
end
# ...
end
To define a child supervisor, we could use:
Supervisor.child_spec([MyServer.child_spec([])], strategy: :simple_one_for_one,
name: MySup)
Notice how the start_link function is gone. It is in no way more complex than
the current status quo - there are still only two functions, but we don't need
to know if the process is a worker or a supervisor. Additionally, there's only
one place, where we define what module the behaviour will be using.
Furthermore, it addresses the issue of defining a "start_link" function - we
never call it explicitly, it's not a callback of any behaviour, and seems quite
random as a name (the BIF to start a process is called spawn_link). Where is it
coming from? You need to know that start_link is this special snowflake of a
function. In this proposal we explicitly call the child_spec function in the
supervisor, and directly enter the behaviour's callbacks. It's very clear to
trace the path of the code and understand what is happening - in the supervisor
we call the MyServer.child_spec/1 function, which calls the
GenServer.child_spec/2, which causes us to enter the behaviour with init/1 -
there are no hidden defaults anywhere, no magic function names, and no macros
(!). We should also consider, requiring calling the child_spec function
directly in the children specification, instead of defaulting to calling
child_spec([]) - it will have similar discoverability issues that current
start_link has, and is only slightly more verbose.
[1] https://github.com/elixir-lang/elixir/pull/5676
[2] https://www.infoq.com/presentations/Simple-Made-Easy
--
You received this message because you are subscribed to the Google Groups
"elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/elixir-lang-core/851e8f39-19da-4749-8437-f848434d2877%40Spark.
For more options, visit https://groups.google.com/d/optout.
