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

configures production as explained in guides for Heroku #23

Merged
merged 4 commits into from
Apr 15, 2021
Merged

configures production as explained in guides for Heroku #23

merged 4 commits into from
Apr 15, 2021

Conversation

jaimeiniesta
Copy link
Contributor

Hi! Related to #22 I think I've found the cause, it looks like config/prod.exs needed some attention as per this guide.

@doomspork
Copy link
Member

Howdy @jaimeiniesta! 🎉 I think we want some of this PR but not all of it. Some of the production configuration occurs at runtime in the releases.exs file here: https://github.com/elixirschool/school_house/blob/master/config/releases.exs#L17-L25

I had to remove the force_ssl: [rewrite_on: [:x_forwarded_proto]], option because of an issue between Cloudflare, Heroku, and Phoenix. With the current configuration between them all, this option results in an infinite loop redirecting to SSL. (Termination happens before Phoenix).

I'll revisit the Cloudflare rules and see if that's the culprit. Until I can do that we'll need to drop that line.

@doomspork doomspork added elixir This work requires Elixir changes enhancement New feature or request labels Apr 15, 2021
@doomspork doomspork self-requested a review April 15, 2021 13:48
@jaimeiniesta
Copy link
Contributor Author

Sounds good to me!

Copy link
Member

@doomspork doomspork left a comment

Choose a reason for hiding this comment

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

@jaimeiniesta with this suggested change I think we should be safe to merge. The domain needed to be updated and we can't use the force_ssl option until I can clean up Cloudflare which appears to impact some other things so may take a little more effort.

config/prod.exs Outdated Show resolved Hide resolved
@jaimeiniesta
Copy link
Contributor Author

LGTM

config/prod.exs Outdated Show resolved Hide resolved
@doomspork doomspork self-requested a review April 15, 2021 16:21
@doomspork
Copy link
Member

I need to look into Heroku pipelines for testing out PRs. I'll try to carve out time this week or next to do that.

@doomspork doomspork merged commit 7982414 into elixirschool:master Apr 15, 2021
@doomspork doomspork mentioned this pull request Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
elixir This work requires Elixir changes enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants