Skip to content
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

Remove Provider.Watch #45

Closed
aeneasr opened this issue Nov 30, 2020 · 4 comments · Fixed by #46
Closed

Remove Provider.Watch #45

aeneasr opened this issue Nov 30, 2020 · 4 comments · Fixed by #46

Comments

@aeneasr
Copy link
Contributor

aeneasr commented Nov 30, 2020

Method Watch(func(event interface{}, err error)) error is actually not used by Koanf (unlike Load and ReadBytes) itself and serves no purpose except for adding bloat to providers who do not support Watching (e.g. env or flags). As an implementer of my own Provider I want to use a different watch mechanism with channels (instead of a callback) which means I have to implement two watchers, or not use channels.

aeneasr added a commit to aeneasr/koanf that referenced this issue Nov 30, 2020
@knadh
Copy link
Owner

knadh commented Dec 1, 2020

The file provider Watch(). While the interface{}, err callback is a bit awkward because it's modeled primarily for filesystem events, it's still is generic enough for any sort of event triggers.

A Provider is meant to be generic, so while async updates might be irrelevant to env and flags, it may be relevant to other config sources like HTTP, Redis, PubSub, Consul etc. Having that one extra function on the interface that can be left empty is not really bloat (in any quantifiable way either) but a fair compromise for a universal Provider interface, I would say.

@aeneasr
Copy link
Contributor Author

aeneasr commented Dec 1, 2020

But the interface method is not used by Koanf (the struct) itself - it serves thus no purpose. See also: https://mobile.twitter.com/davecheney/status/942593128355192832?lang=en

The file provider's Watch method is called directly (see README)!

If we are expecting to call the watch (or whatever other) methods on the provider structs directly, it should be removed IMO.

@knadh knadh closed this as completed in #46 Dec 1, 2020
@knadh
Copy link
Owner

knadh commented Dec 1, 2020

You're right. koanf internally has no use for Watch() and it is used externally either way. I mis-read it the first time. Merged #46

@aeneasr
Copy link
Contributor Author

aeneasr commented Dec 1, 2020

Thank you for listening to my ideas! :)

knadh added a commit that referenced this issue Jul 17, 2024
knadh added a commit that referenced this issue Jul 17, 2024
knadh added a commit that referenced this issue Jul 17, 2024
knadh added a commit that referenced this issue Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants