ex_aws
ex_aws copied to clipboard
ex-aws
Use AWS_REGION and AWS_DEFAULT_REGION env var for default region
More and more tools support either AWS_REGION or AWS_DEFAULT_REGION env vars.
I think it would make sense for ExAws to consider supporting them as well.
AWS_REGION is the most common I've seen, but official CLI uses AWS_DEFAULT_REGION.
Supporting both would be ideal, with AWS_REGION taking precedence over AWS_DEFAULT_REGION.
So, if both are present, AWS_REGION is used. If AWS_DEFAULT_REGION only is present, use it. Otherwise fallback to current behavior ("us-east-1").
IMHO, ex_aws as a lib do its work, application embedding it should handle your concern by configuring it with something like:
config :ex_aws,
access_key_id: [{:system, "AWS_ACCESS_KEY_ID"}, :instance_role],
secret_access_key: [{:system, "AWS_SECRET_ACCESS_KEY"}, :instance_role],
region: [{:system, "AWS_REGION"}, :instance_role]
@ook yes, I do it like that. But as ex_aws reads AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY by default, I think it should read AWS_REGION as well. Or stay a lib and read no env var at all.
It's the fact that it was working with env var, until I hit a region dependent method, which took me a bit to debug. I had to read the source code to realize AWS_REGION was ignored.
I think we should follow elixir practice, is it common for an elixir library to configure itself automagically with env vars? I wonder if @josevalim has any position on this. I really think we should aim for consistency and predictability.
It is likely best if handled inside the library because then it will work with releases. env vars in config files are mostly a no-no.
To be precice, I am not speaking of supporting the {:system, "MYENV"} syntax, but if the lib itself should lookup env var without any configuration, or if the library should require a tuple like {:system, "MYENV"} to lookup MYENV.
To summarize, is it good practice for the following to be implicit in an elixir lib:
config :mylib, myvalue: [{:system, "MYENV"}]
Or should we require the lib user to specify this config line in it's config.exs file.
@kuon that's what I've tried to answer above: {:system, "MYENV"} in config files is not a good practice, so if the lib can do it, it should do it, so users don't have to.
Thanks for the precision, I think it's an important one, as this practice seems to be common.
@josevalim are there good examples somewhere of doing this pattern differently?
@kuon the pattern that Jose is talking about is, as best I can tell, just simply doing
defmodule MyApp do
def start(_, __ do
Application.put_env(:ex_aws, :host, System.get_env("DESIRED_ENV_VARIABLE")
end
end
@josevalim would you say that's correct here?
@benwilson512 @josevalim Yeah I get that, but then the lib (exaws) should drop magic auto read of env var and let the application handle it, no?
@kuon yes, that is exactly what is being proposed
@josevalim This may not be the place, but as I've been messing with this I'm beginning to wonder if it is gonna end up promoting a Rails initializer style of configuration. One of the things that has been nice about mix config heretofore is that it has provided a nice single place to manage configuration.
A very substantial percentage of configuration values need to be configurable by environment variables, at least for libraries like ExAws and Phoenix. What I fear is that is that answers to the question of "Where does config value X get set" go from "check mix config" to "one of many modules". The "many" here being one of the many places in your code that gets run on application boot.
@benwilson512 Some sort of DSL for centralized Application.put_env ? Honnestly I'd like it too, I have been using conform but feels more like a way to provide "sysadmin" config to an app, which has some caveats and is not as nice as env var, which are used everywhere (heroku, docker, …).
@josevalim This may not be the place, but as I've been messing with this I'm beginning to wonder if it is gonna end up promoting a Rails initializer style of configuration. One of the things that has been nice about mix config heretofore is that it has provided a nice single place to manage configuration.
Yes, that's one of the concerns we have so far about this approach. However, that's arguable the best way to do it here because you can have retry mechanisms and the logic is properly encapsulated in entities such as tasks (that will have the job of setting the proper application env keys).
Although, I believe for the particular case of ex_aws, it seems those environment variables are known across projects, so it would be worth to read from them by default and encapsulate the fallback logic in something like ExAWS.Config?
@ryanwinchester I would really appreciate it if you would make comments only about the actual issue here. This is not a general Elixir questions forum. This is the second off topic question, and this one isn't even about this library.
I have removed the off topic comments so we can keep this issue focused.
@benwilson512 I think we should just decide if defaulting to env var for config is a practice we want to keep. The issue about supporting {:system, "MYENV"} is separate.
My vote goes for:
- Keep defaulting to env var for config
- Include the region vars as in my original issue
This would be backward compatible.
If you think defaulting to env var is bad practice, introducing a config DSL or anything is way out of the scope of this issue.
@benwilson512 Any decisions on this? There seem to be some edge cases around loading the config and supporting the region. I'd happily contribute a PR if there is a decision how to proceed with this.
For now let's go ahead and support both via the existing :system mechanism, PR welcome!
Any movement on this or the PR above? I'm relatively new to Elixir and ex_aws...the proposed behavior is consistent with other AWS libraries so this would be a very nice change to pull in.
The current behavior of ignoring the region env var is confusing for those coming from different ecosystems.
