Use java system properties instead of dotenv external library

bowbahdoe / caveman

Clojure Web Framework

https://caveman.mccue.dev

12 stars 8 forks source link

Use java system properties instead of dotenv external library #12

Closed souenzzo closed 2 weeks ago

souenzzo commented 3 weeks ago

Replacing start-env with this code

(defn start-env
  []
  (let [p (System/getProperties)
        dot-env (io/file ".env")]
    (when (.exists dot-env)
      (.load p (io/reader dot-env)))))

We are able to replace these Dotenv/.get calls with (System/getProperty "POSTGRES_USERNAME") and (Long/getLong "PORT")

PROS:

One less library
out-of-the-box parsing
~easy default values: (Long/getLong "PORT" 8080)~ (dotenv also support default values)
it is possible to use long names app.server.port=8080 (short names like PORT are also supported)
Property reader supports both port: 8080 and PORT=8080 syntaxes
Easy REPL overwrite: (System/setProperty "PORT" "3030")
Easier to interact with java API's like org.slf4j.simpleLogger.defaultLogLevel=info

CONS:

Bit more code

bowbahdoe commented 3 weeks ago

You missed some cons:

If you print it out it is giant. It makes it really hard to visually inspect the system. Also it makes it now possible to leak info in a println at any part of your codebase that takes in the system

As compared to

The fact that it actually mutates a global resource is less than ideal.
Supports divergent syntaxes.

There are cons with .env files in general I am puzzling through how to give advice on. At work we use https://www.doppler.com/ and there is no "maybe accidentally committable .env" lying around. Before that we passed around a file through side channels and just made sure it was loaded into the terminal environment. If I can properly describe that then there is no need for any loader code. Its just (System/getenv ...)

The thing that is pushing me in that direction is nvd-clojure and clj-watson. Those both want an environment variable to have creds for pulling in the vulnerability database. If .env isn't loaded in the terminal thats a little annoying. You'd have to source .env && ...

bowbahdoe commented 3 weeks ago

I guess to note another approach - instead of environment variables passing around a full (maybe nested) config file is possible.

The reason I don't want to recommend that is

Now that you have nested maps you need to understand structure of those maps
People will be tempted to go down a pointless rabbit hole of spec-ing that map or in other ways making orchestrations