-
Notifications
You must be signed in to change notification settings - Fork 51
Add 404 page #427
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
Add 404 page #427
Conversation
@CGBassPlayer Looks good so far, any additional changes needed to make this functional as the 404 then? Wondering if this should be draft....? Ready for final review? |
This should be set from the Hugo side of things. I think the configuration for the nginx should be checked to see if it is setup to automatically redirect to the 404 page when the site has a 404. |
^ Anyone willing to tackle this part? |
I'm guessing we need to create an nginx config file to copy into the container? I can try it if everyone else is busy, but it would need someone who actually knows nginx to review it as I am pretty unfamiliar. I also would not be able to get to this until this weekend or beginning next because I am away for a work conference |
Converting back to draft for now. I will into the nginx config stuff in the meantime. If anyone has info on it, please let me know! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything else LGTM, after the changes are accepted and the nginx.conf
is removed.
My only concern for the solution of us using nginx is if traefik will try and use it's own 404 page instead of allowing nginx to serve the 404 page. I'd imagine it'd default to nginx first (because it's essentially the "application"), but I don't personally know which one it'll use.
@ironicbadger, any thoughts on ⬆️?
Co-authored-by: elreydetoda <10230166+elreydetoda@users.noreply.github.com>
Co-authored-by: elreydetoda <10230166+elreydetoda@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Well based on @ironicbadger 's testing in prod I think this LGTM. @CGBassPlayer mark it ready for review and I'll happily merge it 😁
Fixes #388
I am not sure website nginx container is setup to automatically load the 404. But Hugo has a small note that points to nginx's docs about the error code handling