Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better support for proplist-based config, caching variables from such configs #14

Closed
wants to merge 2 commits into from
Closed

Conversation

si14
Copy link

@si14 si14 commented Mar 27, 2012

I've added an option {only_kv, Boolean} to confetti:use/2 so it can control that config that was passed consists only from one proplist. Example:

[{foo, "foo value"},
 {bar, "BAR value"},
 {test, [{test1, 1}, {test2, 2}]}].

I've also added confetti_cache module and confetti:fetch_cache/1 function, so configs like this can be cached in process dict and fetched from there by key.

@aerosol
Copy link
Member

aerosol commented Mar 27, 2012

Hi, and thanks for the contribution.

Let's get back to what we've discussed today on IRC ✌️
It is unclear to me, why not just provide a validator function. Consider the following:

%% example_kv.conf
{key, {val}}.
{key2, val2}.
{key3, [val3, val4]}.

and an appropriate validator fun:

validate_proplist(Conf) ->
    F = fun({K,V}) -> {K,V}; (_) -> throw(error) end,
    {ok, [F(Term) || Term <- Conf]}.  

On a side-note, I don't get why example_kv.conf wraps everything with a list,
so bare consult won't really see this as proplist.

This is the first problem I've got with this. Another one is that you don't
consider an empty list as a valid proplist, therefore your client cannot fall
back to any defaults.

Confetti is meant to be used with several providers at once. I assume that
a system consists of different applications, also different configuration areas
that can co-exist separately even within one application.
Let me give you an example of this: your HTTP server can be configured with
URL routes, debug levels and request throttling rules.
One shouldn't be forced to keep these in one configuration file,
especially when throttling rules are updated frequently and the routing rules
don't really change during the application lifecycle. Therefore - many config files,
and many providers. Erlang has no fear of spawning processes, right?
And that brings us to your cache module.

  1. A key foo in config1.conf will interfere with foo in config2.conf.
    This is no good as it completely ignores the "providers" idea.
  2. I don't understand the purpose of the process dictionary here. What are the
    gains?
  3. Why even cache?

Helpers allowing you to get the exact key are definitely handy though.
Just don't forget the ✨ providers ✨.

Let me know your thoughts. The use case that led you to this might give me
a clue too. Thanks

A.

@si14
Copy link
Author

si14 commented Mar 29, 2012

Yeah, your points make sense. I was thinking about different problem: handling configuration itself. For now Confetti ignores this stuff:

%% User is responsible for implementing detailed
%% config value getters.

I'm not sure that I like it (too much reinventing the wheel in every project); this pull request was an attempt to solve the problem of accessing configuration variables. Not so good, as it turns out.

@aerosol
Copy link
Member

aerosol commented Mar 29, 2012

What I use to ease the pain of getting the exact value is a macro defined as follows:

-define(CFG, fun(Key,Provider,Default) ->
                     Terms = confetti:fetch(Provider),
                     proplists:get_value(Key, Terms, Default)
    end).

It's probably a bit ugly and can be implemented in a more efficient manner, but as long as it's configuration, I assume it's not being fetched too frequently. Otherwise, if you want fast frequent fetches, what you probably need is a key-value store, perhaps something Redis-like?

@aerosol
Copy link
Member

aerosol commented Mar 29, 2012

My common practice is too add a wrapper module too, that keeps all the validator functions and the actual use call. It might be a good place to provide the getters as well.

@aerosol
Copy link
Member

aerosol commented Apr 19, 2012

Follow up in #17

@aerosol aerosol closed this Apr 19, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants