[ENV-40] debounce refresh requests with quietperiod #10172
Conversation
ndeloof
requested review from
a team,
nicksieger,
StefanScherer,
ulyssessouza,
glours,
milas and
laurazard
and removed request for
a team
Codecov ReportBase: 73.89% // Head: 72.79% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## v2 #10172 +/- ##
==========================================
- Coverage 73.89% 72.79% -1.11%
==========================================
Files 2 2
Lines 272 272
==========================================
- Hits 201 198 -3
- Misses 60 62 +2
- Partials 11 12 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
pkg/compose/watch.go
Outdated
| case <-ctx.Done(): | ||
| return | ||
| case service := <-input: | ||
| timer.Reset(interval) |
There was a problem hiding this comment.
Since we don't know if the timer has fired here, we need to stop it/drain first:
if !timer.Stop() {
<-timer.C
}
timer.Reset(interval)
pkg/compose/watch.go
Outdated
| needRefresh := make(chan string) | ||
| eg.Go(func() error { | ||
| debounce(ctx, quietPeriod, needRefresh, func(services []string) { | ||
| fmt.Fprintf(s.stderr(), "Updating %s after changes were detected", strings.Join(services, ", ")) |
There was a problem hiding this comment.
| fmt.Fprintf(s.stderr(), "Updating %s after changes were detected", strings.Join(services, ", ")) | |
| fmt.Fprintf(s.stderr(), "Updating %s after changes were detected\n", strings.Join(services, ", ")) |
(Missing \n)
| "gotest.tools/v3/assert" | ||
| ) | ||
|
|
||
| func Test_debounce(t *testing.T) { |
There was a problem hiding this comment.
You might take a look at Go libs out there to help with controlling timers deterministically in tests, as this is likely to become a problematic/failing test in CI otherwise
Tilt uses this library fairly extensively: https://github.com/jonboulle/clockwork
It provides a clock, which is either the real one (i.e. pass calls through to time.Time) or a fake one where you can control when/how it advances
ndeloof
force-pushed
the
debounce
branch
4 times, most recently
from
7b3905f
to
4137940
Compare
ndeloof
force-pushed
the
debounce
branch
2 times, most recently
from
cb7e7c1
to
f67ddc1
Compare
ndeloof
force-pushed
the
debounce
branch
2 times, most recently
from
fe4e453
to
acbae37
Compare
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
What I did
As we capture fs events, collect them to list services which need to be updated, with debounce and a quiet period