> > 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. >
The best place for dynamic configuration is inside the init/1 function. We have recently added an init/1 function to Ecto.Repo and we will add one to Phoenix.Endpoint before 1.3 is out. So the plan is to push people towards loading those values inside init/1. So I think this proposal will help towards the proper direction, as it will be one less wrong place to put runtime configuration. > 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. > I want to point out that you forgot to consider the worker/supervisor helpers and the parent supervisor in your analysis of how it works today (before this proposal). 1) __MODULE__ is repeated in worker/supervisor and start_link. 2) You need to know if it is a worker/supervisor. And 3) you need to understand the flow of args from worker/supervisor to start_link and to init. In other words, if you do not want to generate the child_spec automatically, which is one of the reasons for this proposal, then it is going to be as repetitive as it is today (not worse). Something that I did not mention is that we will likely add functions such as GenServer.child_spec/2 to help cases where folks need to generate child_spec/2 manually: def child_spec(arg) do GenServer.child_spec(__MODULE__, args: [arg]) # :id, :type, etc are also options end which also solves 1 and 2. Edit: I can see that you propose something similar as well. :) > Additionally we could define the child spec in a way that does not require > the start_link function. > We have discussed this extensively and there are some unresolved issues. For example: 1. How would you pass the :name to your GenServer? 2. What about all other GenServer.start_link/3 options? 3. How will you start the server in tests or anywhere else if it has no start_link function? Our main concern with mixing child_spec and start_link is that it would ultimately mix options and configurations that apply at distinct stages. Even if we introduce something such as GenServer.child_spec(module, arg, start_opts, child_opts), that solves 1 and 2, we still need to find good answers for 3. For tests, we could provide start_child/1 that starts a process under a supervisor for that specific test. But we still need to handle ad-hoc cases. Should we say processes can only be started under a supervisor? Would that be enough? > 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). > spawn_link and start_link have different return signatures. Notice your proposal won't make start_link disappear as well, since they still need to exist as an entry point in any abstraction such as Agent, GenServer, etc. For example, if we say you can only start processes under a supervisor per point 3 above, we still need a mechanism to quickly start a supervisor in IEx, which will likely be Superivsor.start_link. 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. > If you call child_spec directly in the children specification, then we are going to have the same problems as today in that you are no longer able to perform hot code upgrades as the configuration is loaded outside of the supervisor, which is one of the issues this proposal aims to solve. Of course we would like to require as little as possible when implementing behaviours but there are many trade-offs involved in the process. In any case, thanks for the counter proposal. It brings very good points Michał! I would suggest for you to revisit your proposal and consider the following points: * How should GenServer.child_spec look like if we want to pass both start_link options and child_spec options? * How to start processes if we don't have a start_link function. Is the assumption they can only be supervised enough? (unless you call the underlying start_link function in the abstraction directly) * You claim we will be able to get rid of start_link but I don't think that's true, at least not for abstractions such as GenServer and Supervisor. How does that impact your proposal? * Finally you say there are no hidden defaults but everything you hide between GenServer.child_spec is a hidden default. Someone will eventually have learn about it when they need to write their own child_spec. Does it change anything? Let us know how it will progress. Meanwhile we will also discuss the points above between us too. We will ping you if we come up with any new development. -- 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/CAGnRm4%2Bvr8OWgJ2JMyjtCM3aoRaeRjVK1zUhprAO57rBSjF4Bg%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
