Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Users should be able to change most config values at runtime #715

Merged
merged 5 commits into from
Nov 24, 2022

Conversation

geofflane
Copy link
Contributor

Permissions are used at compile time, but other config variables should be read at runtime so that they can be overridden at runtime vs compile time. This allows for 12 Factor type deployment

This is intended to fix: #706

Existing tests pass, but I'm not really sure how to write a test for this exact issue.

  • Application.put_env in a test doesn't trigger the error.
  • Tried adding a test.exs and a runtime.exs with different values but couldn't trigger it either.

Happy to get some help on how to test that and update this PR

Other config variables should be read at runtime so that they can be overridden at runtime vs compile time.
This allows for 12 Factor type deployment
@geofflane geofflane requested a review from a team as a code owner November 23, 2022 16:00
@yordis
Copy link
Member

yordis commented Nov 24, 2022

Hey, do you mind adding an entry here https://github.com/ueberauth/guardian/blob/master/CHANGELOG.md and changing the mix.exs version?

Copy link
Member

@yordis yordis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for the changelog updates.

Thank you so much for the work.

@yordis
Copy link
Member

yordis commented Nov 24, 2022

@geofflane did you get to test this in your app at least? I agree it is a bit tricky to test for sure.

@geofflane
Copy link
Contributor Author

geofflane commented Nov 24, 2022

did you get to test this in your app at least? I agree it is a bit tricky to test for sure.

Yes, I did test locally and 2.3.0 fails to start, but with this change the app will start with a changed secret_key value.

@yordis yordis merged commit d8c87e3 into ueberauth:master Nov 24, 2022
@yordis
Copy link
Member

yordis commented Nov 24, 2022

💜 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to set secret_key in runtime.exs
2 participants