basho / cuttlefish

never lose your childlike sense of wonder baby cuttlefish, promise me?
Apache License 2.0
205 stars 124 forks source link

Better Translation API #83

Closed joedevivo closed 10 years ago

joedevivo commented 10 years ago

This is yet another attempt to resolve #79, assuming I finally understand the issue. I appreciate the solution presented in #80, but I'm going to suggest that maybe it doesn't go far enough.

The original idea for translations worked like this: {translation, Mapping, Fun} Mapping: Was the KVCesque location in the app.config that this thing belonged Fun: The return of this fun() was put in that mapping location.

Then, it was up to the schema writer to write a Fun that returned the value for that location. You could write a fun with case statements and _ -> clauses to return the default, but maybe that's not enough.

Here's the collection of changes I'm considering:

Translation Defaults

Add a proplist to the translation in schema with the ability to add a default value e.g.

{translation,
 "riak_api.http",
 fun(Conf) ->
        HTTP = cuttlefish_util:filter_by_variable_starts_with("listener.http", Conf),
        [ IP || {_, IP} <- HTTP]
 end,
 [
   {default, []}
 ]
}.

Cuttlefish Schema Writer's API

One thing I've been considering for a while is a schema writer's API. Take the function referenced above: cuttlefish_util:filter_by_variable_starts_with/2. There should be one module with these types of functions in them cfish for brevity on the schema side. The cfish versions of these functions would throw errors on things like notfound or whatever.

This would also make an easy "go to" schema writer's documentation reference.

Better Error Handling at Translation Runtime

Right now we try translations and if they error, return undefined. Instead we could return the specified default. We could also do this based on the error, so maybe return the default on notfound which is desirable behavior. But if there's a different kind of error, we should abort the cuttlefish startup and make the user aware of the error.

For example, if you're trying to take the ceiling of a string, I'm not really satisfied that returning the default and starting up is the right thing to do.

Stricter Translation -spec

@licenser dropped the {ok, Value} stuff on us in PR #80 but, I don't think it's enough. We can move to the {ok, Value} semantic, but I'd like to add some {error, Message} to that party. This way in the "ceiling of a string" example, we could include try... catch clauses in translations, and then turn the output into something human readable.

joedevivo commented 10 years ago

I'm happy with how this has been resolved in subsequent pull requests.