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).
> --
> ,,,^..^,,,
