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

Richer functions and more principled top-level parameterization #147

Closed
sparkprime opened this issue Mar 28, 2016 · 16 comments
Closed

Richer functions and more principled top-level parameterization #147

sparkprime opened this issue Mar 28, 2016 · 16 comments
Labels

Comments

@sparkprime
Copy link
Contributor

Context

Jsonnet's original design used object inheritance (mixins) as the primary abstraction mechanism. Since then, users have indicated that in many cases they prefer abstracting via functions. Therefore, this proposal adds several features to make Jsonnet functions more productive. These new features are intended to be conceptually and syntactically compatible with Python. Additionally, an alternative to std.extVar() is proposed which allows top-level parameterization of configurations without creating the equivalent of a global variable.
#1) Add default arguments

Function parameters can have default values. To the function body, such parameters are no different to regular parameters.

$ jsonnet -e '(function(a, b, c=3) a + b + c)(1, 2)'
6

The default expression is bound in the outer scope. E.g.:

$ jsonnet -e 'local a = 3; (function(a, b, c=a) c)(1, 2)'
6

#2) Named arguments

Function call syntax is extended to allow calling a function with named arguments. All positional arguments must be given before all named arguments.

f(b=2, a=1)
f(1, b=2, c=3)

If no parameter exists by that name, an error is raised:

f(1, b=2, d=3)
RUNTIME ERROR: No such parameter "d"

#3) Top-level functions

Previously a Jsonnet file evaluating to a function would be rejected:

$ jsonnet -e 'function() 42'
RUNTIME ERROR: Couldn't manifest function in JSON output.

In this proposal such a function would be called with no arguments.

$ jsonnet -e 'function(a=2) 21 * a'
42

This only works if the function is callable with no arguments:

$ jsonnet -e 'function(a) 21 * a'
RUNTIME ERROR: Expected 1 argument, got 0.

And, only one level of function expansion is performed:

$ jsonnet -e 'function(a=2) function() 21 * a'
RUNTIME ERROR: Couldn't manifest function in JSON output.

To provide an alternative to std.extVar() that is not globally scoped (i.e., requires top-level parameters to be threaded through a large config), new CLI flags are added to provide arguments to top-level functions.

$ jsonnet -e 'function(a, b, c=3) a + b + c' --tla-code a=1 --tla-code b=2
6

Top-level functions can therefore have parameters without default arguments, and if no argument is provided on the CLI then an error is raised:

$ jsonnet -e 'function(a, b, c=3) a + b + c' --tla-code a=1
RUNTIME ERROR: Expected 2-3 arguments, got 1.

#4) CLI flag cleanup

The CLI flags have grown organically and are now somewhat messy. Here is a the current state:

+----------------+--------------+----------------+-----------------+
| Flag           | Kind of data | Source of data | Accessible via  |
+----------------+--------------+----------------+-----------------+
| -V --var       | string       | commandline    | std.extVar(...) |
| -E --env       | string       | environment    | std.extVar(...) |
| -F --file      | string       | file           | std.extVar(...) |
|    --code-var  | jsonnet      | commandline    | std.extVar(...) |
|    --code-env  | jsonnet      | environment    | std.extVar(...) |
|    --code-file | jsonnet      | file           | std.extVar(...) |
+----------------+--------------+----------------+-----------------+

The meaning of the flags is not clearly indicated by their names. Thus, the long forms of these flags is renamed along with the addition of a new dimension to the space (top-level func vs extvar). The result is longer but more organized:

+--------------------+--------------+----------------+-----------------+
| Flag               | Kind of data | Source of data | Accessible via  |
+--------------------+--------------+----------------+-----------------+
| -V --ext-str       | string       | commandline    | std.extVar(...) |
| -E --ext-str-env   | string       | environment    | std.extVar(...) |
| -F --ext-str-file  | string       | file           | std.extVar(...) |
|    --ext-code      | jsonnet      | commandline    | std.extVar(...) |
|    --ext-code-env  | jsonnet      | environment    | std.extVar(...) |
|    --ext-code-file | jsonnet      | file           | std.extVar(...) |
| -T --tla-str       | string       | commandline    | Top-level func  |
|    --tla-str-env   | string       | environment    | Top-level func  |
|    --tla-str-file  | string       | file           | Top-level func  |
|    --tla-code      | jsonnet      | commandline    | Top-level func  |
|    --tla-code-env  | jsonnet      | environment    | Top-level func  |
|    --tla-code-file | jsonnet      | file           | Top-level func  |
+--------------------+--------------+----------------+-----------------+
@sparkprime sparkprime added the rfc label Mar 28, 2016
@sparkprime
Copy link
Contributor Author

@roconnor @davidzchen @mikedanese feedback is appreciated :)

@sparkprime
Copy link
Contributor Author

@copumpkin

@davidzchen
Copy link
Contributor

This seems like a lot of command line flags. I think it would be better if we could cut down on the number of flags rather than increasing them.

Can we use the same set of --ext-* flags for top-level functions instead of creating a separate set of flags for top-level functions?

@sparkprime
Copy link
Contributor Author

Yeah I originally was going to get rid of the -env ones on the grounds that you can just use --ext-str VAR=${VAR}, but then I remembered that environment variables are more secure than commandline arguments because they're hidden from other users on the system. So each of those unfortunately has a unique purpose that is not subsumed by any of the others. Explicit vars are convenient, env vars are safe, and file is for large data.

How about we do --ext-str FOO to mean --ext-str-env FOO, i.e. if there is no = (due to K=V) then treat it as that mode of entry.

What you suggest is interesting. I'd like to be able to keep the extVar, both for backwards compatibility and because in some cases I think it would be useful for things that should be omnipotent, like the version of the software you're configuring, or whatever. So it'd be necessary to have both top level arguments and extVar in flight at once. Maybe we could do this though, with only 4 functions:

--var-str E:K=V
--var-str T:K=V
--var-str-file E:K=V
--var-str-file T:K=V
--var-code E:K=V
--var-code T:K=V
--var-code-file E:K=V
--var-code-file T:K=V

We could also default the T: mode of this.

And the environment variable versions (power user feature):

--var-str E:K
--var-str T:K
--var-code E:K
--var-code T:K

@sparkprime
Copy link
Contributor Author

Another simplification is to drop the str forms and do everything with code. But that requires double escaping in bash, which is annoying.

@sparkprime
Copy link
Contributor Author

/cc @ghodss and @huggsboson a bunch of stuff here you might find interesting

@sparkprime
Copy link
Contributor Author

also /cc @jbeda @chrisleck

@huggsboson
Copy link
Contributor

we have a few places where named args and default args would improve things. Right now we're using single arg functions that take objects and a "getOrElse" function for optional args with defaults. Would love to replace them.

@sparkprime
Copy link
Contributor Author

Good to hear! With this proposal you can't have the default value of one param depend on the others, which you would be able to do with your current approach. This is because I based it on Python:

>>> def foo(x, y=x): print y
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'x' is not defined

Would that be painful? It is something I'm on the fence about right now.

@huggsboson
Copy link
Contributor

All of our current uses merely specify a default literal, so it wouldn't be painful given our existing cases.

@sparkprime
Copy link
Contributor Author

OK, will stick with that semantics then. C++ is the same:

test.cpp:4:20: error: local variable ‘x’ may not appear in this context
 int f(int x, int y=x)

@huggsboson
Copy link
Contributor

speaking of int. Any interest in statical or dynamically checked type assertions on parameters?

@copumpkin
Copy link

I can't speak for current uses of jsonnet, but in Nix at least we have several cases where it was beneficial to have parameter defaults refer to other parameter values. I can try to dig up some examples if helpful.

Overall I like the proposal though.

@sparkprime
Copy link
Contributor Author

Diverting type assertions discussion to #153

@sparkprime
Copy link
Contributor Author

@copumpkin Yes please!

@sparkprime
Copy link
Contributor Author

The one thing that changed from this design in the implementation is that the default arguments can refer to other params (forwards and backwards). This turned out to be easy to both define and implement, as well as useful in some cases. The semantics is equivalent to mutually recursive let:

local f(x=[y[0], 2], y=[1, x[1]]) = x;
f()

Compare to:

local
    x=[y[0], 2],
    y=[1, x[1]];
x

both of which produce:

[
   1,
   2
]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants