Skip to content

Conversation

@nilsocket
Copy link
Contributor

@nilsocket nilsocket commented Oct 31, 2025

This pull request closes issue: #340

Add WatchWithContext() func for following providers:

  • appconfig
  • consul
  • etcd
  • file
  • nats

Other modifications:

  • In file package calls to filepath.Clean() are removed as filepath.EvalSymlinks() will call Clean() on return.
  • Helper function, inputFromConfig() is written and called along with Provider... funcs.
    As it's possible to call Watch() and then Load() which would cause issues.

@nilsocket nilsocket changed the title add WatchWithCtx() funcs for various providers where possible add WatchWithContext() funcs for various providers where possible Oct 31, 2025
@knadh knadh self-assigned this Nov 30, 2025
@knadh
Copy link
Owner

knadh commented Dec 13, 2025

Thanks for the PR @nilsocket. Apart from changes to file.go which are simple, I'm unable to comprehend the changes to appconfig, nats etc (as I've never used those providers myself). Since this PR refactors the constituents of the existing Watch() functions, I'm unsure how to go about making sure that existing Watch() functionality is unchanged.

@nilsocket
Copy link
Contributor Author

nilsocket commented Dec 13, 2025

I have modified Watch() in order to limit duplicate code, should I modfiy it?
So, existing Watch() methods aren't touched.

@knadh
Copy link
Owner

knadh commented Dec 13, 2025

Duplicating so much code wouldn't make sense. Are you in a position to test out Read(), and Watch() for these providers?

@nilsocket
Copy link
Contributor Author

Yep, but I need some time for that (mostly a month).

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 this pull request may close these issues.

2 participants