Add a fs watcher based reloader for PathOrContent#17
Add a fs watcher based reloader for PathOrContent#17douglascamata wants to merge 7 commits intoefficientgo:mainfrom
Conversation
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
saswatamcode
left a comment
saswatamcode
left a comment
There was a problem hiding this comment.
LGTM! 🌟
Maybe you can add a section in docs.go and update the readme https://github.com/efficientgo/tools#module-githubcomefficientgotoolsextkingpin?
|
@saswatamcode suuuure, working on it. 💪 |
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
|
@saswatamcode when do you think we can merge this? 😬 |
bwplotka
left a comment
bwplotka
left a comment
There was a problem hiding this comment.
Nice! Good overall, just some usability suggestions.
| // Copyright (c) The EfficientGo Authors. | ||
| // Licensed under the Apache License 2.0. | ||
|
|
||
| // Taken from Thanos project. |
| Path() string | ||
| } | ||
|
|
||
| // PathContentReloader starts a file watcher that monitors the file indicated by pathOrContent.Path() and runs |
There was a problem hiding this comment.
Let's make a note this can run forever if not cancelled?
Also I wonder if it wouldn't be cleaner to make it sync. Otherwise it looks like a function that just leaks things when you are not careful. So either make it sync so others would run it in go routine or return closer?
| // the debounce timer. By default the debouncer timer is 1 second. | ||
| // To ensure renames and deletes are properly handled, the file watcher is put at the file's parent folder. See | ||
| // https://github.com/fsnotify/fsnotify/issues/214 for more details. | ||
| func PathContentReloader(ctx context.Context, fileContent pathOrContent, debugLogger logger, errorLogger logger, reloadFunc func(), debounceTime time.Duration) error { |
There was a problem hiding this comment.
| func PathContentReloader(ctx context.Context, fileContent pathOrContent, debugLogger logger, errorLogger logger, reloadFunc func(), debounceTime time.Duration) error { | |
| func StartPathContentReloader(ctx context.Context, fileContent pathOrContent, debugLogger logger, errorLogger logger, reloadFunc func(), debounceTime time.Duration) error { |
I think we need verb to tell what's going on 🤔
|
@bwplotka this PR is now severely outdated. I made the reloader independent of fs watchers due to problems with k8s secrets and configmaps (see thanos-io/thanos#6503). Will update the PR here when I get some time. |
4 participants
Signed-off-by: Douglas Camata 159076+douglascamata@users.noreply.github.com
Extracting the filesystem based reloader for
PathOrContentfrom thanos-io/thanos#5673 into theextkingpinpackage here so that it can be more easily reused by other projects and even in Thanos itself (where it was originally created).