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

time: allow to inject per-module or per-thread now() #516

Closed
ash2k opened this issue Oct 27, 2023 · 3 comments · Fixed by #517
Closed

time: allow to inject per-module or per-thread now() #516

ash2k opened this issue Oct 27, 2023 · 3 comments · Fixed by #517

Comments

@ash2k
Copy link
Contributor

ash2k commented Oct 27, 2023

This is a follow up for #435.

Global var is still there in the time module:

var NowFunc = time.Now

I'd like to be able to customize what now() returns on a per-thread basis. Alternatively, I'd like to be able to instantiate a new time module with injected now() (or both).

Thank you!

@adonovan
Copy link
Collaborator

I would accept a PR to make the now function consult a starlark.Thread-local variable (of type func() time.Now) before falling back to the standard time.Now implementation.

@ash2k
Copy link
Contributor Author

ash2k commented Oct 27, 2023

Thank you. I'll open a PR soon.

Do you want me to keep the existing package var for backward compatibility or remove it?

@adonovan
Copy link
Collaborator

We can't remove it, but we can deprecate it and recommend that people use the new mechanism instead, which avoids global state.

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