Make MuxManagerStartDelay configurable#231
Conversation
27c3781 to
126cee7
Compare
liam-lowe
left a comment
There was a problem hiding this comment.
I think there is either a simpler or more optimal approach here - either feed through standard config route, or implement dynamic "readiness"
| // GlobalPolicy is the process-wide policy singleton. | ||
| var GlobalPolicy = &Policy{ | ||
| MuxManagerStartDelay: time.Minute, | ||
| } |
There was a problem hiding this comment.
do we want to introduce a global policy here - I don't want to conflate our existing configuration approach
| // initial connections before serving. Accepts Go duration strings | ||
| // (e.g. "30s", "1m", or "0s" to skip the wait entirely). When the | ||
| // field is omitted, the in-process default (time.Minute) is used. | ||
| MuxManagerStartDelay *Duration `yaml:"muxManagerStartDelay,omitempty"` |
| @@ -1,3 +1,4 @@ | |||
| muxManagerStartDelay: "0s" | |||
There was a problem hiding this comment.
will 0 lead to race conditions - otherwise why was it set?
There was a problem hiding this comment.
only for testing. In test code, the value is ready set to 0
|
The main purpose of this PR is to speed up local testing by avoiding the fixed 1-minute wait, following the existing pattern used in the test code. Longer term, I’d like to remove this delay entirely — potentially via dynamic “readiness” as you suggested — so this setting is intended as a temporary workaround. Thefore, I’d prefer to avoid wiring it into the official configuration path. |
- common/global_policy.go: drop the now-redundant doc comments on Policy and UpdateMuxManagerStartDelay; the field-level docs already cover the semantics. - config/config_test.go: merge TestDurationUnmarshalYAML_NestedStructAndOmitted into TestDurationUnmarshalYAML by always unmarshaling through a wrapper struct. Adds an "absent_field" row that the old top-level scalar form couldn't express. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
9b6c7cd to
8e403ff
Compare
What was changed
Why?
not wait for local test.
Checklist
Closes
How was this tested: