tonerdo / dotnet-env

A .NET library to load environment variables from .env files
MIT License
427 stars 50 forks source link

support parsing .env and returning a Dictionary<string,string> instead of modifying environment #40

Closed jamesmanning closed 3 years ago

jamesmanning commented 4 years ago

in Microsoft.Extensions.Configuration, ConfigurationBuilder has the ability to load configuration from a dictionary.

Because of that, it would be nice to support using DotNetEnv to parse the .env file (and all the support it has there, including the trimming, quoted values, etc) and return it as a dictionary then allow the caller to use that dictionary.

Currently because it doesn't support returning a dictionary instead of modifying the environment, a .net core test project wanting to use it must switch their <Project Sdk= value from Microsoft.NET.Sdk to Microsoft.NET.Sdk.Web so they can use AddEnvironmentVariables

jamesmanning commented 4 years ago

@rogusdev would you be ok with this kind of change? If so, would you prefer it as new static Parse calls on the existing Env class that returned Dictionary<string,string> or surface as a new class (presumably wanting to keep Parser internal)?

I had started down the path of adding 4 static Parse calls to Env (path, lines, stream, default - same overloads as Load) and added tests for them but wanted to find out what you would prefer before opening a PR.

rogusdev commented 4 years ago

Sure, I would welcome a PR like that! It would just be a new parsetodict method or something -- maybe rename the old methods to parsetoenv and add deprecation attributes on methods named the old way for backwards compatibility that point to parsetoenv

rogusdev commented 3 years ago

As noted on your PR #42 , this will be handled by the upcoming dotnet-env 2.0, which you can see a preview of here: https://github.com/rogusdev/dotnet-env/commit/1204d059c8a199cca55ee6eb7b9ee1707946dbba (I just need to fix up all the old tests, which will take a bit) -- the solution is a new loadoption to determine if env vars should be parsed while loading or not (meaning should there be any writing of env vars during parsing so that any $ENVVAR references can subsequently be loaded in later .env lines) -- and all load functions now return the IEnumerable<KeyValuePair<string,string>> which matches the signature of the configuration builder extension you linked (and is convenient in this new approach already).

rogusdev commented 3 years ago

Actually, before I do too much more work, is it your desire that: 1) loading/parsing does not modify the env vars as your issue title says or 2) variables in .env files are not interpolated/populated/inflated such that they remain as $ENVVAR or 3) both/either, depending

The trick is an example like:

TRICKY_VAR="$EXISTING_VAR"

where EXISTING_VAR in the env when parsed is equal to hello

Do you want to have Load return a dict / ienumerable of kvps that has: 1) "TRICKY_VAR" => "hello" (the value of the referenced/interpolated env var in the .env assignment value) or 2) "TRICKY_VAR" => "$EXISTING_VAR" (the actual string with a dollar sign and name of the variable, not its value)

And then separately, afterwards, should: 1) env | grep TRICKY_VAR return nothing, because no env vars were modified? or 2) env | grep TRICKY_VAR return `TRICKY_VAR=hello" (or "TRICKY_VAR=$EXISTING_VAR", depending on the first question) because the env var was in fact set?

I'm not really familiar with your use case and I can imagine both being theoretically possible but I don't want to add feature bloat that no one wants.

rogusdev commented 3 years ago

New PR is up for this, would love your review: https://github.com/tonerdo/dotnet-env/pull/44

jamesmanning commented 3 years ago

I don't personally need/use any variable interpolation, my target .env just has regular foo=bar pairs, I'm never going to use the interpolation support. As a user, I just want a library that will just parse the .env file, I don't want it touching the environment at all. Separate Load and Parse methods were kind of the intent, either "parse the file, return to me what you find" or "parse the file, load it up into the current environment, optionally return it". I prefer dict to enum kvp just so I can dict["FOO"] as a consumer more easily and not have to worry about what's returned having duplicated keys

jamesmanning commented 3 years ago

I'd also prefer something that would automatically look up the directory hierarchy for the .env like some similar libraries do, but I can live without that. 😄

rogusdev commented 3 years ago

Can you create a separate issue for the directory hierarchy request, and elaborate there?

rogusdev commented 3 years ago

The new setEnvVars option does what you want then. Is that acceptable for you?

rogusdev commented 3 years ago

@jamesmanning is this what you mean about looking up the directory hierarchy for .env files? https://github.com/rogusdev/dotnet-env/pull/2 I have not noticed other projects providing this feature however, and I am mildly concerned that it could lead to mysterious undesired behavior for some users. Can you point me at existing projects using this?

jamesmanning commented 3 years ago

@jamesmanning is this what you mean about looking up the directory hierarchy for .env files? rogusdev#2

Yes, exactly

I have not noticed other projects providing this feature however, and I am mildly concerned that it could lead to mysterious undesired behavior for some users. Can you point me at existing projects using this?

https://github.com/bolorundurowb/dotenv.net#autoconfig

rogusdev commented 3 years ago

Version 2.0.0 is out with this included! https://www.nuget.org/packages/DotNetEnv/2.0.0/

You can return a Dictionary with ToDictionary() on the result from load, so the full version of what you want (with traversing up for .env) is:

DotNetEnv.Env.TraversePath().Load().ToDictionary();
jamesmanning commented 3 years ago

That’s awesome! Thanks @rogusdev !!