lightbend / config

configuration library for JVM languages using HOCON files
https://lightbend.github.io/config/
6.12k stars 968 forks source link

Enable programmatic overriding of environment variables #797

Open przemek-pokrywka opened 11 months ago

przemek-pokrywka commented 11 months ago

This new API allows users to easily test how environment variables substitutions would work in their complex configurations, with:

Besides environment variables, the new API also lets one substitute system properties and standard error stream, everything in a scoped way (nothing is set globally, only per caller thread).

Caveat: the substitutions work only in the current thread, however, this should fit 99.9% of users, since it is rather unheard of, that one does multi-threaded computations during the configuration loading/resolving process.

References #796

lightbend-cla-validator commented 11 months ago

Hi @przemek-pokrywka,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

https://www.lightbend.com/contribute/cla

ThijsBroersen commented 11 months ago

+1

mkurz commented 11 months ago

This PR likely will never be merged.

https://github.com/lightbend/config#maintained-by

The "Typesafe Config" library is an important foundation to how Akka and other JVM libraries manage configuration. We at Lightbend consider the functionality of this library as feature complete. We will make sure "Typesafe Config" keeps up with future JVM versions, but will rarely make any other changes.

przemek-pokrywka commented 11 months ago

Thanks for chipping in @mkurz, unfortunately, I am aware of that, yet I am trying to make people realize the utility of the change, especially since it is small and pretty well-contained.

Dear @lightbend maintainers, please consider merging this change, as no good workaround for the issue exists. Why: if one wanted to temporarily replace the JVM's private copy of environment variables through reflection, it will always be exposed to the risk of 1) future JVMs changing their internals and 2) concurrent access to the environment from other threads

yantonov commented 10 months ago

Even if it's a direct fix for the existing functionality new patches are not applied.

https://github.com/lightbend/config#maintained-by

It was discussed previously, and the conclusion is always the same.

ennru commented 9 months ago

Thank you for trying to improve this library. I understand your wish to improve testability of your code, but we decided not to merge your suggestion. We are releasing a new version containing #798 which addresses security concerns, though.

przemek-pokrywka commented 9 months ago

I understand your wish to improve testability of your code

Dear @ennru, dear @lightbend maintainers, not only of my code but also of code maintained by teams, to which the people interested in that issue belong. And, by virtue of transitive dependencies, of code that uses zio-config and similar libraries. So if your decision was based on the affected audience, I would really expect that it's wider than just me, and I would encourage you to reconsider it.

Modern standards of testability are much higher than they were at the time of the conception of this library. You, as the steward, have a unique opportunity to keep the library relevant.

yantonov commented 8 months ago

Here https://github.com/lightbend/config/pull/798 extended functionality was merged (which is completely non-critical), although a) it's explicitly stated that the library is feature-complete (https://github.com/lightbend/config#maintained-by); b) but bugs for existing functionality and explicitly stated and therefore expected behavior have not been fixed for years. Great!

dobrynya commented 7 months ago

+1

przemek-pokrywka commented 7 months ago

Dear @ennru, dear https://github.com/lightbend maintainers,

As correctly pointed out by @yantonov at https://github.com/lightbend/config/pull/797#issuecomment-1778614070, feature completeness should not mean, that the users should cope with the existing problems until they move to an alternative. Weak testability is a problem for anyone, who does non-trivial (or production-critical) things with configuration, and you know very well, that the library gives one great power, linked to great risk sometimes.

As the responsible stewards of the library, which still enjoys great popularity within the ecosystem, please do consider merging these changes.