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

Enable programmatic overriding of environment variables #797

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

przemek-pokrywka
Copy link

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

  • no need for ugly reflection hacks (JVM provides no way to alter env. variables)
  • and without being forced to run their tests sequentially (as even with the hack, the env. variables are global).

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

within defined scope.
Also works for system properties,
and for standard error stream.
@lightbend-cla-validator
Copy link
Collaborator

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
Copy link

+1

@mkurz
Copy link

mkurz commented Aug 19, 2023

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
Copy link
Author

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

This comment was marked as duplicate.

@ennru
Copy link
Member

ennru commented Oct 17, 2023

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
Copy link
Author

przemek-pokrywka commented Oct 17, 2023

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
Copy link

yantonov commented Oct 25, 2023

Here #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
Copy link

+1

@przemek-pokrywka
Copy link
Author

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

As correctly pointed out by @yantonov at #797 (comment), 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.

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.

7 participants