chrismccord / mailgun

Elixir Mailgun Client
MIT License
194 stars 95 forks source link

System.get_env #34

Open plamb opened 8 years ago

plamb commented 8 years ago

I may be totally looking at this wrong and would love to be corrected.

It looks like if you did the config like this:

config :my_app, mailgun_domain: "https://api.mailgun.net/v3/mydomain.com",
                mailgun_key: System.get_env("MAILGUN_API_KEY")

It's going to pull the environment variable at compile time since the config happens in a macro? The problem, I'm running into is that at runtime the env variable may be different than the compile time one (if there's even a compile time one there).

speg commented 8 years ago

I've been battling this off and on for a few days too. I've been trying to follow this with the Heroku deploy guide. But the environment variables don't work there. Everything seems to work okay locally though? I don't even know how I can configure the Mailgun client without using the keys directly since the environment variables don't seem to be available at compile time.

Oh, I see. You have to explicitly state which config vars you want to export for compile time.

https://github.com/HashNuke/heroku-buildpack-elixir#specifying-config-vars-to-export-at-compile-time

vitalis commented 8 years ago

I have the same issue with Heroku, if the config is set during compile time it cannot be changed by changing ENV variable, the app must be recompiled. Will be glad to help with this issue.

geofflane commented 8 years ago

System.get_env("MAILGUN_API_KEY") is a compile time value. That gets converted by Mix.Config into a sys.config file at compile time and the value of the environment variable at compile time is used.

On top of that, the config macro in this libray is expanded at compile time as well, and caches the value from the Mix.Config/sys.config. So the value that will be in it is the one that is set at compile time only.

Short answer: The configuration of this app doesn't support changing the configuration at runtime which is kind of broken.

rradz commented 8 years ago

@geofflane My little patch should help with that.