Thanks, Paul. Your answers are good for me. -- ,,,^..^,,,
On Fri, May 22, 2015 at 11:17 PM, Paul Davis <[email protected]> wrote: > On Fri, May 22, 2015 at 2:37 PM, Alexander Shorin <[email protected]> wrote: >> Hi Paul, >> >> Great proposal! Replied inline to preserve context. >> >>> Bikeshed First >>> -------------- >>> >>> I have no idea what we'd call this. We could repurpose the >>> couch_plugins app conceivably or make something new. For the the >>> purposes of this document I'll call it couch_epi (for extensible >>> plugin interface) and hopefully that's terrible enough someone will >>> think of a better name for the actual application. >> >> +1. And suggest call them as extensions, not plugins, to avoid confusion. >> >> >>> === Automatically Discoverable === >>> >>> The biggest thing here is that I don't want to require a change to a >>> default.ini or similar to enable or disable specific functionality >>> when we can already signify that by having the application present or >>> not. This is both for groups that may want to add new Erlang >>> applications to a release as well as anyone that wants to run a >>> minimal/embedded Couch. These are both obviously advanced uses but I >>> think are important given the number of ways that CouchDB is being >>> used. >> >> I think here you mixed two different cases: default.ini and friends >> are for user side configuration. Releases are vendor side. If user >> want to turnoff some extension and we may guarantee that it's possible >> with no harm. Like currently we have HTTPS support which is optional. >> > > This was a bit muddled. But I was trying to say that for users that > want to rearrange a full release or include part of the CouchDB code > base in a different release this system would aim to allow that. Your > point about HTTP support is a good one, that would still be > configurable in the config and not part of this system. > >> >>> === Minimize the apps that need to be started for tests === >>> >>> This one I think should be obvious to anyone that's been writing unit >>> tests lately. There are some often silly places where we require >>> applications be started just to run some tests. For example, places >>> where we may want to call a function that's been instrumented and >>> requires couch_stats to have knowledge about the stat. >> >> Technically speaking, we have three kind of tests: functional, >> integration and unit ones. If you writing the first and second ones, >> you have to run all the required apps. For the last one: just mock all >> the side effects as log/stats/io are. That's not an issue at all. >> > > Good point. > >> >>> Data Centric Examples >>> --------------------- >>> >>> For a concrete example, lets consider couch_stats. Any application >>> that wants to record metrics through the standard couch_stats app >>> could add an entry in its supervision tree with something like: >>> >>> { >>> appname_stats, >>> {couch_epi_data_source, start_link, [ >>> appname, >>> {epi_key, {couch_stats, definitions}} >>> {priv_file, "couch_stats.cfg"} >>> ]}, >>> permanent, >>> 5000, >>> worker, >>> dynamic >>> } >>> >>> (...skip...) >>> >>> Function Centric Examples >>> ------------------------- >>> >>> Hopefully its obvious that given the data centric approach we could do >>> something quite similar for functions (given that an MFA is just a >>> small bit of data that we can use to invoke any function). >>> >>> Though ovbiously we'd like to be able to have a bit more of a useful >>> API for clients so that we don't require all function based extensions >>> to have to reimplement that function invocation code. >>> >>> The first thing that would change would be to provide a different type >>> of supervision tree entry to indicate this. Off the top of my head >>> this would look something like such: >>> >>> { >>> appname_funcs, >>> {couch_epi_functions, start_link, [ >>> appname, >>> {module, appname_funcs_mod} >>> ]}, >>> permanent, >>> 5000, >>> worker, >>> dynamic >>> } >>> >>> (...skip...) >> >> All looks good, but don't you think that this will cause us to >> implement yet another dependency resolution issue? >> >> Assume my extension need to define own metrics for couch_stats and a >> handler for chttpd. While I put couch_stats and chttpd in my deps list >> I'm feel fine about their presence and if they are missed or changed >> their API I will be instantly warn about. With couch_epi proxy that >> would be a problem as it hides target function call and there is no >> way to check during compilation if that function even exists with the >> right arity. How this problem could be solved? >> > > I'd consider couch_stats and chttpd to be two different use cases. > couch_stats is a dependency because if you're defining stats then > you're most likely calling into the couch_stats API. For chttpd you'll > more than likely only use chttpd functions when being invoked by > chttpd, thus if you want that app in a context where chttpd doesn't > exist there's not an issue. > > As to function signatures I would just rely on convention as is done > everywhere else. While you can get xref support its not something we > generally run cross application, and even if so, we're just checking > for MFA rather than any sort of type on each argument. > >> Another question is what about resolution order and how to avoid >> attempts to call some hook before the underlying app is actually >> started and registered itself on epi server? >> > > I'm not sure what you mean by this. In my version you'd end up with > either nothing happening or the call not being made if it weren't > registered. The extensions that Ilya suggested my suffer from such a > thing but I'd like to avoid those for these sorts of reasons. > > On the other hand, if you mean, what would happen if we called a > couch_stats function before getting the definition installed, I'd lean > on the supervision tree startup order to get that. Ie, we'd just list > the couch_epi_data_source child before anything else so the stats get > registered before the app starts up. Though of course we're also > pretty bad in places where we try and call into apps before their > started because we don't have proper dependencies specified everywhere > though hopefully that'd be solvable by reevaluating things if it > causes a problem (and its not something introduced by this approach > since we still have the couch_stats startup scanning async for > definitions). > >> -- >> ,,,^..^,,,
