caketop / python-starlark-go

🐍 Python bindings for starlark-go 🐍
https://python-starlark-go.readthedocs.io/
Apache License 2.0
20 stars 7 forks source link

Upgrade starlark-go (20240311180835-efac67204ba7) #204

Open romain-h opened 6 months ago

romain-h commented 6 months ago

Upgrade the Go starlark package to the latest one. This involves a rewrite of the test_configure.py since the starlark resolver configuration is evaluated part of the exec. Using global config vars is deprecated and should be replaced. Not part of this PR as it would involve a breaking change of the API.

jordemort commented 6 months ago

Thanks!

I'm trying to understand this; it looks like configure_starlark now clears the interpreter's state, which makes it necessary to call exec again after every configure... is that correct?

If so, that seems like kind of a breaking change to this module's API anyway; maybe it would be better to go ahead and change the API to get rid of global config vars altogether (and/or maybe add some sort of Context object to hold defaults as a replacement for them) and bump the major version. What do you think?

romain-h commented 6 months ago

Yes indeed, this is already introducing a breaking change.

In this case, as you said, we should remove configure_starlark completely and expose the FileOptions introduced here. We then update Starlark's methods with an extra named keyword argument file_options (optional).

For example, applied to the eval method:

from starlark_go import Starlark, FileOptions

s = Starlark()

file_options = FileOptions(allow_recursion=True)
x = s.eval("7.7", file_options=file_options)