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

Add --yaml-stream #117

Merged
merged 1 commit into from
Mar 8, 2016
Merged

Add --yaml-stream #117

merged 1 commit into from
Mar 8, 2016

Conversation

sparkprime
Copy link
Member

@mikedanese
Copy link
Contributor

My comment would be that it seems like a lot of change (and modifies very internal components). Unlike multi this could be implemented in a couple lines of jsonnet and -S:

{
    yamlStream(arr)::
        std.foldl(function(old, obj) old + "---\n%s\n...\n" % [obj], arr, ""),
}

The requirement seems very similar to the requirement to output .ini, but the implementation and api is quite different.

I'll call a "finalizer" a single final function that get's called on the output of a jsonnet program. I've seen three use cases for "finalizers" so far:

  1. yamlStream
  2. manifestIni
  3. truth.run (the unit testing main).

I'm sure there will be more. Is it worth it or possible to design a reasonable and generic API for finalizers? That said, if the api existed, it would be easy to move the cli API over to it and even the c API deprecating the functions added here.

@davidzchen
Copy link
Contributor

What is the use case for this?

@sparkprime
Copy link
Member Author

@davidzchen The use case is basically Kubernetes. But the YAML stream format is a standard.

@mikedanese Good point, this could at least be implemented in the desugarer and by enabling -S with -y.

What you're calling a finalizer is exactly what I've been calling manifestation though I think. It's a final step that converts a value to a string. OK the semantics actually says convert a Jsonnet value to a JSON value by forcing thunks, bindings objects, and stripping hidden fields. But It could just as easily say "call the std.manifestJson function and get a string".

So should we be able to specify a jsonnet file on the commandline and that would define the manifestation function?

@sparkprime
Copy link
Member Author

jsonnet --manifest=std.manifestJson -e "{}"  # Also the default.
{}
$ jsonnet --manifest=3 -e "{}"
3
$ jsonnet --manifest=std.manifestYamlStream -e '[{}, {}]'
---
{}
---
{}
...

@sparkprime
Copy link
Member Author

So you desugar jsonnet --manifest=M -e E to M(E)
This is an error if it doesn't return a string and the string is not escaped in any way.

The old -S behavior is now --manifest=function(x)x

@sparkprime
Copy link
Member Author

Here's a challenge with this though: If you do

local X = {
    a: 1,
    b: 3/self.a,
};

{
    A: X {
        a: 1,
    },
    B: X {
        a: 0,
    },
    C: X {
        a: 2,
    },
}

Then you get

RUNTIME ERROR: Division by zero.
        test.jsonnet:3:8-15     object <X>
        test.jsonnet:(10:8)-(12:5)      object <anonymous>
        During manifestation

Which has the very useful piece of contextual information test.jsonnet:(10:8)-(12:5) that lets us know where we instantiated the bad X from.

However if you use std.manifestJson (which I just wrote), you get:

dcunnin@tyrion:~$ jsonnet -eS 'std.manifestJson(import "test.jsonnet")'
RUNTIME ERROR: Division by zero.
        test.jsonnet:3:8-15     object <anonymous>
        std.jsonnet:744:50-53   thunk <v>
        std.jsonnet:720:16      thunk <a>
        std.jsonnet:954:29      thunk <x>
        std.jsonnet:954:20-30   thunk <ta>
        std.jsonnet:956:33-34   thunk <a>
        std.jsonnet:956:13-39   function <anonymous>
        std.jsonnet:720:16-24   function <aux>
        std.jsonnet:744:46-84   thunk <array_element>
        std.jsonnet:148:21-26   thunk <a>
        ...
        std.jsonnet:954:20-30   thunk <ta>
        std.jsonnet:956:33-34   thunk <a>
        std.jsonnet:956:13-39   function <anonymous>
        std.jsonnet:148:21-34   function <aux>
        std.jsonnet:153:17-62   function <aux>
        std.jsonnet:157:13-33   function <anonymous>
        std.jsonnet:747:17-35   function <aux>
        std.jsonnet:748:9-26    function <anonymous>
        std.jsonnet:716:27-59   function <anonymous>
        <cmdline>:1:1-39

Which lacks that information.

This problem already existing with std.manifestPython of course but JSON is the common case here.

Some solutions:

  1. Proceed with PR as is (at least for now)

  2. Make std.manifestJson a builtin that calls the current manifestation code (this is a bit different to just std.toString(v) because it escapes a string at the top level and also introduces newlines and indenting).

  3. Allow catching the error and adding contextual information to it at each recursion point. This also makes it possible to unit test libraries that are intended to fail. Then the error would look like
    RUNTIME ERROR: Division by zero while manifesting $.B.b

@sparkprime
Copy link
Member Author

Catching and rethrowing would have to preserve the stack trace as well, so there's some design work there.

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

Successfully merging this pull request may close these issues.

None yet

4 participants