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

Can't tell what starlark Builtin cannot be converted to Python #143

Closed
colindean opened this issue Jan 23, 2023 · 4 comments · Fixed by #167
Closed

Can't tell what starlark Builtin cannot be converted to Python #143

colindean opened this issue Jan 23, 2023 · 4 comments · Fixed by #167

Comments

@colindean
Copy link
Contributor

colindean commented Jan 23, 2023

I can't tell which property of the final output of my Starlark script is not convertible.

$ python3 test.py test.star
Traceback (most recent call last):
  File "/Users/Z003XC4/Source/Target/TargetOSS/python-poetry/test.py", line 12, in <module>
    print(s.eval("main([])"))
starlark_go.errors.ConversionError: Don't know how to convert *starlark.Builtin to Python value

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/Z003XC4/Source/Target/TargetOSS/python-poetry/test.py", line 14, in <module>
    print(e.backtrace)
AttributeError: 'ConversionError' object has no attribute 'backtrace'

Test code:

import sys

from starlark_go import Starlark
from starlark_go.errors import EvalError, ConversionError

s = Starlark()

with open(sys.argv[1], 'r') as input_file:
    s.exec(input_file.read())

try:
    print(s.eval("main([])"))
except (EvalError, ConversionError) as e:
    print(e.backtrace)

The test code is working with simple dictionaries so it might be something else fancier I'm (unknowingly) doing that's not resolving to something convertible.

@jordemort
Copy link
Contributor

Can you share the Starlark that triggers this as well?

@jordemort
Copy link
Contributor

It looks like you're trying to return a reference to a function somehow? A Builtin is a Go function that is callable from Starlark. I'd like for this module to support returning Callables back to Python but it doesn't at this time.

@colindean
Copy link
Contributor Author

colindean commented Jan 23, 2023

I introduce a print() of the final output and tracked it down to basically this:

"slack": {"needs": <built-in method keys of dict value>, 

and this is the minimum reproduction:

def stage_slack_notify(stage_names):
    return {"name": "slack", "needs": stage_names}

def main(ctx):
    build_stages = {"something": [], "something_else": []}

    stages = [build_stages] + [stage_slack_notify(build_stages.keys)]
    final = {
        'version': '1',
        'stages': stages,
    }

    print(final)

    return final

Can you spot the error?

Dict.keys() is a function and I forgot (). 🙃 PEBKAC.

But I still feel like there's a better way to communicate the error, perhaps by telling exactly what wasn't able to be converted, with some path to it if possible.

@jordemort
Copy link
Contributor

Keeping track of the path to the value would be doable but maybe kinda ugly. I think we'd have to add a path parameter to every function in starlark_to_python.go and have starlark[Dict|Tuple|Set|List]ToPython append keys to it before passing it along to the next function.

jordemort added a commit that referenced this issue Mar 16, 2023
- Subclass into ConversionToPythonFailed and ConversionToStarlarkFailed
- Make messages much more informative and indicate which key or index failed
- If conversion fails because of a Python exception, add that exception as the source of ours

Closes #143
jordemort added a commit that referenced this issue Mar 16, 2023
- Subclass into ConversionToPythonFailed and ConversionToStarlarkFailed
- Make messages much more informative and indicate which key or index failed
- If conversion fails because of a Python exception, add that exception as the source of ours

Closes #143
jordemort added a commit that referenced this issue Mar 16, 2023
- Subclass into ConversionToPythonFailed and ConversionToStarlarkFailed
- Make messages much more informative and indicate which key or index failed
- If conversion fails because of a Python exception, add that exception as the source of ours

Closes #143
jordemort added a commit that referenced this issue Mar 18, 2023
* Make ConversionErrors more useful

- Subclass into ConversionToPythonFailed and ConversionToStarlarkFailed
- Make messages much more informative and indicate which key or index failed
- If conversion fails because of a Python exception, add that exception as the source of ours
- Fix some minor issues in starlark.c documentation while we're there

Closes #143
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 a pull request may close this issue.

2 participants