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

Upgrade starlark-go (20240311180835-efac67204ba7) #204

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

romain-h
Copy link

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.

Upgrade the Go starlark package to latest one. This involves a rewrite
of the test_configure.py since the starlark resolver configuration is
evaluated part of the exec. This is because using global config vars is
deprecated now. This is not replacing the usage of it yet as it would
involve breaking change of the API.
@jordemort
Copy link
Contributor

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
Copy link
Author

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)

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 this pull request may close these issues.

None yet

2 participants