dry-system icon indicating copy to clipboard operation
dry-system copied to clipboard

Allow env variable prefix and setting namespace to be configured in settings provider

Open jeremiahrose opened this issue 2 years ago • 5 comments

Closes https://github.com/dry-rb/dry-system/issues/253

This PR adds 2 new configuration options to the settings provider source:

  1. prefix: sets the prefix of the environment variables that will be read. E.g with a prefix of APP1_ and a setting named log_level, the settings provider will look for an environment variable named APP1_LOG_LEVEL.

  2. register_as: sets the namespace that the setting will be registered to. This enables settings objects to be organised according to their purpose e.g Container['app.config.database'] and also allows multiple settings providers to be registered.

Update: rebased to latest main

jeremiahrose avatar Nov 02 '22 01:11 jeremiahrose

Thank you for working on this but this has to wait. We're in the process of releasing dry-system 1.0.0 and then Hanami 2.0.0 and it's code freeze now. We'll get back to this PR after we're done with the Hanami release (so, before the end of the month).

solnic avatar Nov 04 '22 06:11 solnic

Thanks @jeremiahrose for this contribution, and thanks @solnic for saying what I was planning to get around to saying too :)

In terms the viability of this feature, I'd definitely be confident in it being a reasonable post-1.0.0 addition. Nothing here e.g. looks like it'd be a breaking change to any of the APIs release to 1.0.0, so it'd be a straightforward expansion/addition of our existing APIs.

However, before merging anything, I expect I'd probably want to have a think about different possible ways of satisfying the end goal here. One thing I worry about with adding built-in support for very specific features (like a variable prefix) is that if we continue that same approach for any other adjustments people want with settings loading, we may end up continuing to glom things on and end up with a hard-to-understand combination of different configs around settings loading (think the homer car, and honestly dry-system is probably already a little like that, so I'd like to be quite intentional in the way we add additional features from here).

For example, another option here could be to instead add support for a single configurable "settings store" (which is something we actually have for Hanami's settings implementation). By default, this would pulling settings straight from the env like we do now. But this would also give us a way to satisfy your needs through providing a custom "prefixed names settings store" that pulls from the env using a provided prefix. This kind of arrangement may ultimately be more flexible to serving different needs without us having to add too much complexity to this part of dry-system.

timriley avatar Nov 04 '22 11:11 timriley

another option here could be to instead add support for a single configurable "settings store" (which is something we actually have for Hanami's settings implementation). By default, this would pulling settings straight from the env like we do now. But this would also give us a way to satisfy your needs through providing a custom "prefixed names settings store" that pulls from the env using a provided prefix.

I guess that could tie in with the current changes quite well, just make the "prefixed names settings store" the default one. I think being able to set environment variable prefixes is a fairly standard feature for an environment loader that most users will want.

jeremiahrose avatar Nov 11 '22 02:11 jeremiahrose

Congrats on the release of dry-system 1.0! Is there anything I can do to help move this card along?

jeremiahrose avatar Dec 18 '22 22:12 jeremiahrose

Just wondering if dry-system is still in code freeze and is there anything I can do to help move this card along?

jeremiahrose avatar Apr 10 '23 22:04 jeremiahrose