messense/rjsonnet-py

Error when using ext vars after previous panic

epwalsh opened this issue · 4 comments

Hey @messense, thanks for this package! We're trying to integrate this into Tango (allenai/tango#505) to replace the jsonnet Python package, but we've run into a weird issue. It looks like it's a bug in the rjsonnet Rust lib, but I wanted to open an issue here in case you have any ideas. It's easiest to explain with an example.

First of all, this works as expected:

import json
from pathlib import Path

from rjsonnet import evaluate_file

key = "hello"
val = "world"
path = Path("/tmp/test_ext_vars.jsonnet")
with open(path, "w") as f:
    f.write(f'{{"{key}": std.extVar("{key}")}}')

print(json.loads(evaluate_file(path.name, str(path.parent), ext_vars={key: val})))

Output:

{'hello': 'world'}

But when you try the same thing after forcing the Rust lib to panic, you now get an error:

import json
from pathlib import Path

from rjsonnet import evaluate_file

# Force a panic.
path = Path("/tmp/test_bad_string.jsonnet")
with open(path, "w") as f:
    f.write(r'{"myRegex": "a\.b"}')
try:
    print(json.loads(evaluate_file(path.name, str(path.parent))))
except BaseException:
    # This is expected, although Rust should probably return an error instead of panicking.
    print(f"error evaluating {path}")

# Same thing as above.
key = "hello"
val = "world"
path = Path("/tmp/test_ext_vars.jsonnet")
with open(path, "w") as f:
    f.write(f'{{"{key}": std.extVar("{key}")}}')

# This now fails with "external variable is not defined".
print(json.loads(evaluate_file(path.name, str(path.parent), ext_vars={key: val})))

Output:

Traceback (most recent call last):
  File "/Users/evanw/AllenAI/tango/tmp.py", line 22, in <module>
    print(json.loads(evaluate_file(path.name, str(path.parent), ext_vars={key: val})))
RuntimeError: external variable is not defined: hello
    /tmp/test_ext_vars.jsonnet:1:11-31: function <std.extVar> call
                                      : field <hello> manifestification

The panic is from https://github.com/CertainLach/jrsonnet/blob/e1fb5e1edb6f4935484bb3073564c8808e7fafac/crates/jrsonnet-parser/src/lib.rs#L107

I think it needs a jrsonnet upgrade to resolve, there are lots of breaking changes in jrsonnet master branch, it will take some time.

Thanks @messense!

The panic is fixed now so you don't have to use except BaseException: anymore, it correctly raises RuntimeError.

Let me know if have any more issues.

Thanks for the quick turn-around!