peburrows / goth

Elixir package for Oauth authentication via Google Cloud APIs
http://hexdocs.pm/goth
MIT License
289 stars 111 forks source link

Fetch config in handle_info instead of init #15

Closed mkaszubowski closed 6 years ago

mkaszubowski commented 7 years ago

If the config is not set, starting the Config genserver will fail right now. Because it's started when the application start, the whole startup process fill fail because of this.

This causes the whole application will fail if goth is added as dependency and the config is not set. This is inconvenient when you want to use it in tests, when it's not really used.

After this change, Config can start even if the config is not set and this fixes the issue.

peburrows commented 7 years ago

Awesome. I actually ran into this issue yesterday when I didn't have anything configured in my tests in an application. I'll give this a look and get it merged as soon as I've validated it.

Thanks!

mkaszubowski commented 7 years ago

Oh, sorry, pushed new commit to my master by accident which updated in this pull request also.

But actually what I'm doing here makes sense for me. Right now, the config expects the json as a string and config :goth, :json, "path" |> File.read! is done at compile time.

This is not a good idea because it's harder to handle it while building the release and configuring it on different hosts. I tried to provide a raw value, but that didn't worked so well (conform escaped the \ character which caused poison to fail).

I think it would be great to be able to use json_path config option and load the file at runtime. This makes it easier to set this up correctly on different host (I was able to get it to work using this way after failing to do so earlier). If you agree with me, let me know, I'd be happy continue working on this.

peburrows commented 7 years ago

Yeah, a json_path config would be useful for sure. We should probably check the json config first, and fallback to json_path if it exists.

That said, the way I normally solve it is to use an ENV variable that has the config, and Goth will load it at runtime if you set via:

config :goth, :json, {:system, "GCP_CREDENTIALS"}
mkaszubowski commented 7 years ago

Sure, using the system ENV can be a solution, but it's not always perfect (as I've said, I'm using conform to manage the configuration, and using ENV would require me to have the config in two places: the usual *.conf file with all the config and the ENV for Goth - this would be harder to maintain and more error-prone). Having the option to use path in the config would certainly help.

I can modify the code and first check for the :json field and the :json_file. I will try to make the changes in the next few days :)

jersearls commented 6 years ago

I'm running into the same issue as @mkaszubowski. I can set the json as an environmental variable, however I'm deploying to EBS, which has character limits to ENV vars and is truncating the json. Meanwhile, Goth loads immediately at runtime before I can call a function to download the json from an outside source. Goth will then error out without the json. Any movement, thoughts on how to solve this?

peburrows commented 6 years ago

@jersearls, I've just put something together that should address this: https://github.com/peburrows/goth/pull/32 should give you the ability to dynamically set config at runtime by implementing an init/1 callback.

I need to write some more tests around it, but give it a try and let me know if you run into anything.

(I'm going to close this PR, as #32 should cover all the cases mentioned here.)

jersearls commented 6 years ago

Awesome! Thanks!

jersearls commented 6 years ago

@peburrows I just wanted to say that this update worked perfectly! Thanks for being so responsive