google / starlark-go

Starlark in Go: the Starlark configuration language, implemented in Go
BSD 3-Clause "New" or "Revised" License
2.31k stars 209 forks source link

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

Closed ash2k closed 11 months ago

ash2k commented 11 months ago

This is a follow up for https://github.com/google/starlark-go/issues/435.

Global var is still there in the time module:

https://github.com/google/starlark-go/blob/22325403fcb3696ebcc542d0e7cc75156d1fdca3/lib/time/time.go#L74

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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