You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This one is kinda extreme, but I think the interface (and the package itself) can be simplified.
The most important function of this package is its parsing of .env files. Compare it to dotenv_parser
The other features are secondary: i.e. reading files, setting system environment values, etc.
The feature which I think is not necessary is the GenServer-based value store: that adds a layer of complexity which I think is not necessary. Apps already have a stateful key/value store in their process dictionary (backed by ETS and available via the Application.get_env functions). Having a large number of ENV variables would unnecessarily bloat the memory because values could potentially get stored inside Application and inside the dotenv process. This duplication brings with it some confusion because now one must reconcile the source of truth and reload etc.
Given that the raison d'être of this package is to load up config at runtime (i.e. when the app starts), I think you could focus the interface on a single function: the one that loads and parses a file (or a list of them) and sets the system environment variables. There would probably be a couple other secondary functions that would exist in order to serve that primary one (e.g. a parsing function and one that controlled how and if variables are set into the system), but the keystone to the entire thing is the function that kicks all of that off, something like:
def load_file(path \\ ".env", opts \\ []), do: # ... returns :ok or :error tuple
def load_file!(path \\ ".env", opts \\ []), do: # ... returns result or raises
def load_files(path \\ [".env"], opts \\ []), do: # ... consider a variant to handle a list
The options should include:
:overwrite? boolean indicating whether or not values parsed from provided .env files should
overwrite existing system environment variables. Default: false
:required_files specifies which of the given files (if any) must be present.
When true, all the listed files must exist.
When false, none of the listed files must exist.
When some of the files are required and some are optional, provide a list
specifying which files are required.
:side_effect an arity 1 function called after the successful parsing of each of the given files.
The default is &System.put_env/1, which will have the effect of setting system environment variables based on
the results of the file parsing.
And possibly a couple other options to facilitate testing.
The followup helper functionality here that would be helpful would be the conversion functions that help modify the string values into integers, booleans, module names etc. but this is secondary to the above.
I realize this idea would represent a significant change to the package as it is currently, but I thought it wouldn't hurt to toss it out here for discussion/consideration.
The text was updated successfully, but these errors were encountered:
This one is kinda extreme, but I think the interface (and the package itself) can be simplified.
The most important function of this package is its parsing of
.env
files. Compare it to dotenv_parserThe other features are secondary: i.e. reading files, setting system environment values, etc.
The feature which I think is not necessary is the GenServer-based value store: that adds a layer of complexity which I think is not necessary. Apps already have a stateful key/value store in their process dictionary (backed by ETS and available via the
Application.get_env
functions). Having a large number of ENV variables would unnecessarily bloat the memory because values could potentially get stored insideApplication
and inside thedotenv
process. This duplication brings with it some confusion because now one must reconcile the source of truth and reload etc.Given that the raison d'être of this package is to load up config at runtime (i.e. when the app starts), I think you could focus the interface on a single function: the one that loads and parses a file (or a list of them) and sets the system environment variables. There would probably be a couple other secondary functions that would exist in order to serve that primary one (e.g. a parsing function and one that controlled how and if variables are set into the system), but the keystone to the entire thing is the function that kicks all of that off, something like:
The options should include:
:overwrite?
boolean indicating whether or not values parsed from provided.env
files shouldoverwrite existing system environment variables. Default:
false
:required_files
specifies which of the givenfiles
(if any) must be present.When
true
, all the listed files must exist.When
false
, none of the listed files must exist.When some of the files are required and some are optional, provide a list
specifying which files are required.
:side_effect
an arity 1 function called after the successful parsing of each of the given files.The default is
&System.put_env/1
, which will have the effect of setting system environment variables based onthe results of the file parsing.
And possibly a couple other options to facilitate testing.
The followup helper functionality here that would be helpful would be the conversion functions that help modify the string values into integers, booleans, module names etc. but this is secondary to the above.
I realize this idea would represent a significant change to the package as it is currently, but I thought it wouldn't hurt to toss it out here for discussion/consideration.
The text was updated successfully, but these errors were encountered: