peburrows
commented
4 years ago If you're interested in this, I would be happy to update the docs and provide a test case.
Yeah, I think this makes sense. If you could add tests around this configuration, I'd be happy to merge this PR.
Regarding where to put this config, I think the application config is the right place for now. I have ideas about potentially refactoring some stuff around config and also process management to allow, for instance, people to more easily run multiple Goth processes with different configs, but until I get around to doing that, I think what you've done here still makes the most sense.
wojtekmach
Hi, thanks for putting this library together! It has worked flawlessly for us for 3 years now.
Yesterday we unintentionally flooded our elixir app with a lot of concurrent requests/workloads. This was on our staging environment where autoscaling for load distribution was disabled for this service. This meant that this one app was very busy. In fact, it would appear, there were so many processes spawning, dying and calling Goth to get a token, the refresh task was not able to refresh the token in time.
Goth currently refreshes a token 10 seconds before it is due to expire. As far as we could gather, the token is valid for 1h, so we thought that we could be more lenient with the refresh window.
TLDR: In this PR we introduce an option to configure the refresh window.
This commit is what we are working with right now, but I guess there are some options to consider regarding where the config should go.
Goth.Configmodule, perhaps similar toactor_email. That config would still be application wide and it would mean one extra call to the Config GenServer.If you're interested in this, I would be happy to update the docs and provide a test case.
Cheers!