-
Notifications
You must be signed in to change notification settings - Fork 14
Add seed flag #98
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
Add seed flag #98
Conversation
pkg/genlib/generator.go
Outdated
// InitGeneratorRandSeed sets rand seed | ||
func InitGeneratorRandSeed(randSeed int64) { | ||
// set rand and randomdata seed to --seed flag (custom or 1) | ||
rand.Seed(randSeed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
math/rand.Seed
is deprecated and it seems the deprecation affects our use case.
I would not rely on the global PRNG and instantiate one with the generator.
Also the rand.Seed
default behaviour changes in Go 1.20:
If Seed is not called, the generator is seeded randomly at program startup.
Prior to Go 1.20, the generator was seeded like Seed(1) at program startup.
Due to this I think we need to ensure to always call Seed with an appropriate default or when using Go 1.20 users of genlib
library may experience unexpected behaviours.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to this I think we need to ensure to always call Seed with an appropriate default or when using Go 1.20 users of
genlib
library may experience unexpected behaviours.
that's happening indeed: since randSeed
, if --seed
is not passed, has a default value of 1
I've replaced rand.Seed
with a customRand = rand.New()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually as @aspacca noticed, in Go 1.19.x rand.Seed
is not deprecated, but in 1.20.0 the method is deprecated and the behaviour changes. Also the Go docs is wrong, as noted here: https://stackoverflow.com/questions/75597325/rand-seedseed-is-deprecated-how-to-use-newrandnewseed
It should be rand.New
not NewRand
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@endorama done!
// InitGeneratorRandSeed sets rand seed | ||
func InitGeneratorRandSeed(randSeed int64) { | ||
// set rand and randomdata seed to --seed flag (custom or 1) | ||
customRand = rand.New(rand.NewSource(randSeed)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
We add a
--seed
flag to feed the rand seed used in pkg and by go-randomdata dependecy.timeNow
andrandSeed
are moved to their init functions in pkg instead of passed to the generator factoriesaws.ec2_logs
assets have been refactored so to achieve reproducible generation given the same--now
and--seed
value