Add support for reading .env files #25

anthonycastelli · 2018-05-12T07:03:15Z

After reading through the chat the past few days, people have wanted a different solution to dealing with config variables and one of the solutions is to use .env files.

Special thanks to @cyphactor for his awesome regex skills on this one. 👍

rafiki270 · 2018-05-12T07:36:53Z

Sources/Service/Environment/Environment+DotEnv.swift

+
+extension Environment {
+    /// Loads the ".env" and the ".env.environment" files if they exist
+    public func loadDotEnv() {


I think this could maybe do with func load(env: String = ".env") or similar instead of hardcoding it?

rafiki270 · 2018-05-12T07:37:28Z

Sources/Service/Environment/Environment+DotEnv.swift

+    }
+
+    /// Loads the specific env file and sets the appropiate environment variables
+    public func loadDotEnvFile(_ filename: String) {


oh, it's here ... func load(dotEnvFile filename: String) would be more "swifty"

rafiki270 · 2018-05-12T07:39:53Z

I really like the idea of having the ability to load an external config file (I am already doing this manually myself) ... just not sure the public interface should a) limit to .env files and b) not liking the method names very much ...

anthonycastelli · 2018-05-12T08:05:39Z

I thought about naming them .vapor files instead of .env, however .env is a standard. After talking in the #development channel, it might be worth writing a custom encoder/decoder for this, but after thinking about it, is it really needed for something this trivial? Maybe @tanner0101 can weigh in on it

rafiki270 · 2018-05-12T08:13:09Z

this is much better, hope this makes it in!

drewdeponte · 2018-05-12T08:44:38Z

Sources/Service/Environment/Environment+DotEnv.swift

            setenv(key, value, 1)
        }
    }

    /// Parse the contents of the .env file into a readable key/value
-    func parseDotEnv(with content: String) -> [String : String] {
+    func parse(with content: String) -> [String : String] {


Now that you removed the DotEnv from the method name there is nothing that tells the caller what type of content it is wanting to parse.

Maybe would should change it to func parse(withDotEnv content: String)?

If this method was in a class specific to dot env file formats I would feel different.

func parse(dotEnv content: String)? not a big fan of with in method signatures, also don't think they much used in vapor?

This is an internal function, so the user shouldn't ever see this function

If with prefixing isn't used much in vapor, func parse(dotEnv content: String) makes the most sense to me even if it is internal. Still wan't whoever the call is (internal/external) to know what the method signature is expecting. I guess the way that would really make me happy but requires more effort and I am not sure makes sense is to have a signature like.

typealias DotEnv = String func parse(content: DotEnv)

drewdeponte · 2018-05-12T08:56:34Z

Sources/Service/Environment/Environment+DotEnv.swift

+
+extension Environment {
+    /// Loads the ".env" and the ".env.environment" files if they exist
+    public func loadEnvironment() {


Also, is there a reason to not just call this load()?

I felt load() was just too broad.

In my opinion it is fine in this case because it is scoped within the context of an Environment instance. So the expected call would look something like env.load(). When I look at env.loadEnvironment() I feel like it is redundant. But, I am cool with whatever just sharing my thoughts.

…e documentation

drewdeponte · 2018-05-14T17:51:55Z

Sources/Service/Environment/Environment+DotEnv.swift

+    /// Loads the specific env file and sets the appropiate environment variables
+    /// Parse the contents of the .env file into a readable key/value
+    /// - parameter environment: The filename of the .env file to load. i.e. `.env`.
+    /// Note: The .env files must be loaced in the root directory.


Typo in loaced should be located I believe.

drewdeponte · 2018-05-14T17:52:55Z

Sources/Service/Environment/Environment+DotEnv.swift

+    }
+
+    /// Parse the contents of the .env file into a readable key/value
+    /// - parameter content: The .env string. This can be multiples lines. i.e. `"USERNAME="Vapor"`


Have an extra leading " in front of USERNAME that shouldn't be there.

drewdeponte · 2018-05-14T17:54:17Z

Sources/Service/Environment/Environment+DotEnv.swift

+    /// Both a generic .env file and an environment specific file are loaded.
+    /// This allows you to set custom credentials to specific environments such as
+    /// production, testing, and development.
+    /// If you use custom environments, those will also be loaded.


Maybe add something explaining that the environment specific ones override the general one.

tanner0101

I like the idea of adding file-based config, but not sure this is the right way to do it. Using the existing environment stuff isn't very type safe or convenient so I don't really want to encourage using it. It would be great if we could utilize Codable and a custom config file decoder to parse these configuration structs. Then the end user could do something like:

.env

FOO=hello
BAR=42

struct MyConfig: Decodable {
    var foo: String
    var bar: Int
}

let config = try INIDecoder().decode(MyConfig.self, file: ".env")
print(config.foo) // "hello"

From here you can add lots of nice conveniences.

let config = try MyConfig.load(ini: ".env")

IMO these components would be more re-usable and valuable on their own, too.

Note: not sure what exact file format this is. Looked like an INI file to me. This could be done with any file format though. :)

tanner0101 · 2018-05-15T01:22:11Z

Sources/Service/Environment/Environment+DotEnv.swift

+    /// Note: The .env files must be located in the root directory.
+    public func load(environment fileName: String) {
+        // Create the file path
+        let url = URL(fileURLWithPath: DirectoryConfig.detect().workDir).appendingPathComponent(fileName)


The DirectoryConfig should be supplied here, or even better, created from services. That way the user can configure a custom working directory.

anthonycastelli · 2018-05-15T01:38:41Z

I started working on a custom decoder for this, but felt like it was just way too much overhead for such a small thing. I'll go back and work on it then. I guess it would be a more ideal and safer solution. This leads me to wonder if it's even best to use this type of config file. Is there something better, or just stick with a plain text key=value system? I'm thinking the latter just cause it's a well known format for things like this.

tanner0101 · 2018-05-15T01:48:05Z

@anthonycastelli coders can seem a bit verbose but they are really not so bad once you learn the tricks. The default fix-it that xcode gives you adds way more methods than you actually need. You should be able to implement this INI coder with just ~10 methods total (since you don't need to support arrays or single value).

I know there's already a Codable impl for YAML: https://github.com/jpsim/Yams. I think one for the .ini/.conf file format would be a greatly appreciated package.

On top of actual coder you just need a thin layer that does the actual file loading. You could make this generic to work with any coder which I think would be ideal. That way people can store their config in whichever file format they want. You could probably get away with using FileManager for this since it's almost always going to happen during boot and performance isn't a huge deal. If you want to make it performant enough to use inside a route closure (not really sure why that would be useful, but who knows what people might want to do) then you should use NIO's NonBlockingFileIO type. Vapor wraps it with FileIO but you could build off of NIO's to make this code more general. It would still be quite easy to hook up to Vapor.

anthonycastelli · 2018-05-15T06:25:22Z

How does something like this work?

.env

# This is a test comment
string="Coding Test" # Another Comment
float=0.3
double=0.2
int=1
uint=12
bool=true

Config Struct

struct TestConfigs: EnvironmentConfig {
    let string: String
    let float: Float
    let double: Double
    let int: Int
    let uint: UInt
    let bool: Bool
}

Decoding

let configs = TestConfigs.load()
print(configs.string) // Coding Test

drewdeponte · 2018-05-15T08:56:57Z

Lets not forget that the intent of loading the config isn't to load it into a structure but to load it into environment variables so that it works as a 12 factor application (https://12factor.net). This allows it to work seamlessly with platforms like Heroku, Docker, etc.

So, in this case would it be Codable that would be read in from the file, and then also coded into environment variables somehow?

vzsg · 2018-05-15T12:40:09Z

There are different readings to the 12factor manifesto.

In my opinion, this line: "The twelve-factor app stores config in environment variables" does not imply that internally the application can only read environment variables.

The application might as well merge environment variables (the requirement) with other strategies to build a global configuration state in memory, then use it to configure the components internally – like Spring Boot does.

drewdeponte · 2018-05-15T17:06:44Z

@vzsg I am less interested in the various interpretations of 12factor manifesto and more interested in the fact that it is relatively common to configure an application via environment variables in 12factor based PaaS systems. Therefore, whatever path is chosen shouldn't prevent that from happening and in my opninion should make it easy if not trivial to support environment variables based configurations.

vzsg · 2018-05-15T17:17:58Z

We're on the same opinion then.

anthonycastelli · 2018-05-15T17:38:14Z

The ability to decode to a custom struct and then set the variables to the env is still there.
configs.setEnvironmentVariables() @cyphactor

The functionality is there, just not set by default.

rafiki270 · 2018-05-16T05:48:04Z

I think you should drop the headers :) ...

Also, please make a separate package with that decoder :))) @anthonycastelli

anthonycastelli · 2018-05-16T06:01:10Z

The whole point of this PR was to incorperate it into Vapor, a standardized way to load config variables. Unless one of the @core members vote no on implementing such thing into Vapor itself, I think it’s best to keep it here.

tanner0101 · 2018-05-16T17:53:36Z

I think it could make sense to include this in Vapor, but the .ini/.conf decoder should be a separate package. Just like the multipart and form-urlencoded coders.

In terms of the base protocol for declaring a decodable config, I think that should go in vapor/vapor, not vapor/service. This package is just for DI so file-based config doesn't make a whole lot of sense in that context IMO.

anthonycastelli · 2018-05-16T17:58:59Z

I don't think I have permission to create a repository on the vapor organization so you'll have to do that @tanner0101. I've gone ahead and closed this in vapor of moving it to vapor/vapor.

tanner0101 · 2018-05-16T18:05:27Z

@anthonycastelli I've created a new repo here: https://github.com/vapor-community/conf-file. You should have admin rights to it. Once that package is complete we can add the vapor/vapor dep and move it over to the vapor org :)

tanner0101 · 2018-05-16T18:06:28Z

Feel free to rename that repo btw. I'm not sure which exact conf file format we should support. Based of this it seems .ini and .conf are commonly referred to as "configuration file" format.

anthonycastelli · 2018-05-16T18:08:23Z

Since it's part of vapor-community, should it be left out of the core Vapor framework then?

Also, the .env format is neither .ini or .conf but either of those can be used.

tanner0101 · 2018-05-16T18:10:58Z

Once that package is complete we can add the vapor/vapor dep and move it over to the vapor org :)

What I meant by that is once the package is complete, we can move it to the official vapor organization.

anthonycastelli · 2018-05-16T18:11:48Z

Okay, gotcha. 👍

anthonycastelli added 2 commits May 11, 2018 23:59

environment: Add support for reading from .env files

7d0c7ac

tests: add .env reading and parsing tests

7966182

rafiki270 reviewed May 12, 2018

View reviewed changes

environment: Clean up the function names

44e48e4

drewdeponte reviewed May 12, 2018

View reviewed changes

anthonycastelli requested a review from tanner0101 May 12, 2018 21:49

environment: Give each funtion cleaer names, as well as more extensiv…

c4d8148

…e documentation

drewdeponte reviewed May 14, 2018

View reviewed changes

environment: Fix a couple of typos

b60e067

drewdeponte reviewed May 14, 2018

View reviewed changes

tanner0101 reviewed May 15, 2018

View reviewed changes

environment: Use a customer decoder to load .env files

5e50f81

environment: Remove a dead file

a56b661

rafiki270 assigned anthonycastelli and unassigned anthonycastelli May 16, 2018

anthonycastelli closed this May 16, 2018

anthonycastelli deleted the dontenv branch May 16, 2018 18:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for reading .env files #25

Add support for reading .env files #25

anthonycastelli commented May 12, 2018

rafiki270 May 12, 2018

rafiki270 May 12, 2018

rafiki270 commented May 12, 2018

anthonycastelli commented May 12, 2018 •

edited

Loading

rafiki270 commented May 12, 2018

drewdeponte May 12, 2018 •

edited

Loading

rafiki270 May 12, 2018

anthonycastelli May 12, 2018

drewdeponte May 14, 2018

drewdeponte May 12, 2018

anthonycastelli May 12, 2018

drewdeponte May 14, 2018

drewdeponte May 14, 2018

drewdeponte May 14, 2018

drewdeponte May 14, 2018

tanner0101 left a comment •

edited

Loading

tanner0101 May 15, 2018

anthonycastelli commented May 15, 2018 •

edited

Loading

tanner0101 commented May 15, 2018 •

edited

Loading

anthonycastelli commented May 15, 2018

drewdeponte commented May 15, 2018

vzsg commented May 15, 2018 •

edited

Loading

drewdeponte commented May 15, 2018

vzsg commented May 15, 2018

anthonycastelli commented May 15, 2018 •

edited

Loading

rafiki270 commented May 16, 2018

anthonycastelli commented May 16, 2018

tanner0101 commented May 16, 2018

anthonycastelli commented May 16, 2018

tanner0101 commented May 16, 2018

tanner0101 commented May 16, 2018 •

edited

Loading

anthonycastelli commented May 16, 2018 •

edited

Loading

tanner0101 commented May 16, 2018

anthonycastelli commented May 16, 2018

Add support for reading .env files #25

Add support for reading .env files #25

Conversation

anthonycastelli commented May 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rafiki270 commented May 12, 2018

anthonycastelli commented May 12, 2018 • edited Loading

rafiki270 commented May 12, 2018

drewdeponte May 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tanner0101 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anthonycastelli commented May 15, 2018 • edited Loading

tanner0101 commented May 15, 2018 • edited Loading

anthonycastelli commented May 15, 2018

drewdeponte commented May 15, 2018

vzsg commented May 15, 2018 • edited Loading

drewdeponte commented May 15, 2018

vzsg commented May 15, 2018

anthonycastelli commented May 15, 2018 • edited Loading

rafiki270 commented May 16, 2018

anthonycastelli commented May 16, 2018

tanner0101 commented May 16, 2018

anthonycastelli commented May 16, 2018

tanner0101 commented May 16, 2018

tanner0101 commented May 16, 2018 • edited Loading

anthonycastelli commented May 16, 2018 • edited Loading

tanner0101 commented May 16, 2018

anthonycastelli commented May 16, 2018

anthonycastelli commented May 12, 2018 •

edited

Loading

drewdeponte May 12, 2018 •

edited

Loading

tanner0101 left a comment •

edited

Loading

anthonycastelli commented May 15, 2018 •

edited

Loading

tanner0101 commented May 15, 2018 •

edited

Loading

vzsg commented May 15, 2018 •

edited

Loading

anthonycastelli commented May 15, 2018 •

edited

Loading

tanner0101 commented May 16, 2018 •

edited

Loading

anthonycastelli commented May 16, 2018 •

edited

Loading