-
Notifications
You must be signed in to change notification settings - Fork 47
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 simple state persistence strategy #1301
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1301 +/- ##
==========================================
- Coverage 50.51% 41.81% -8.70%
==========================================
Files 145 144 -1
Lines 7544 9082 +1538
==========================================
- Hits 3811 3798 -13
- Misses 3066 4711 +1645
+ Partials 667 573 -94
Continue to review full report at Codecov.
|
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.
A nit about the log
} | ||
} | ||
} else { | ||
l.Warn("State won't persist! Set `state.persistEvery` to a positive value") |
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.
Can you set to warn just for the first time or every n?
Otherwise it will spam the log
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.
That's kind of the idea, one should never have this value at 0
. But, I get that for some random test it could be useful. I see a couple things we could do:
- Add a "counter" member variable to
Chain
that helps us print "every n". - Have a RNG and have it print with a 20% probability...
First case I dislike, putting a member variable just for a warn
print is an ugly idea. Second case I like better, but it introduces overhead. What do you think?
Resolves: #1298