Core + Python: Enable cluster periodic checks by default, add python configuration #1089
Conversation
barshaul
force-pushed
the
enable_periodic_checks
branch
from
32b77c2 to
8944ffb
Compare
There was a problem hiding this comment.
I'd like to have a clearer distinction between manual / disabled values.
also, this PR is probably dependent on #1086, since periodic checks will increase these tests flakeyness.
barshaul
force-pushed
the
enable_periodic_checks
branch
3 times, most recently
from
29c4204 to
4c6f3eb
Compare
|
@shachlanAmazon round |
barshaul
force-pushed
the
enable_periodic_checks
branch
3 times, most recently
from
8048ec7 to
f71e024
Compare
| "\nPeriodic Checks: Disabled".to_string() | ||
| } | ||
| }, | ||
| None => format!( |
There was a problem hiding this comment.
usually we only print the configuration values that the user has modified, not defaults. your protobuf representation essentially means that we can't differentiate between a user explicitly choosing the default, and a user who just didn't set the value.
There was a problem hiding this comment.
Internally both options means the same. I can add the enabled option to protobuf but it means that if the status is None or if the user passes enabled status we will do the same, causing both code duplication and less clear code (why having two options with the same output?)
barshaul
force-pushed
the
enable_periodic_checks
branch
2 times, most recently
from
7bdadda to
fd6af6f
Compare
barshaul
force-pushed
the
enable_periodic_checks
branch
from
fd6af6f to
69d9ad3
Compare
No description provided.