-
Notifications
You must be signed in to change notification settings - Fork 202
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
Switch local API dev server to use gunicorn instead of django runserver #2936
Switch local API dev server to use gunicorn instead of django runserver #2936
Conversation
@sarayourfriend Kept it in draft state for now. Are there any other changes I need to make? |
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.
I think this looks good to me. I haven't tested it locally yet, but will do so today. I think we could leave out the --bind
parameter, I expected that to be handled by the gunicorn.conf
, but I could be wrong.
Okay, everything looks good, just three things, only one of which is significant:
|
@sarayourfriend Made the changes you requested and was able to test that static files are not 404ing locally |
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.
LGTM, thanks for the contribution @ashiramin. I've gone ahead and added testing instructions but in the future please add them once a PR is marked ready for review. It makes it much easier for maintainers to review PRs 🙏
@sarayourfriend Thank you for doing this and for reminding me about testing instructions. I'll do this next time :) |
Based on the high urgency of this PR, the following reviewers are being gently reminded to review this PR: @dhruvkb Excluding weekend1 days, this PR was ready for review 2 day(s) ago. PRs labelled with high urgency are expected to be reviewed within 2 weekday(s)2. @ashiramin, if this PR is not ready for a review, please draft it to prevent reviewers from getting further unnecessary pings. Footnotes
|
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.
LGTM, sorry I took so long to review it!
Fixes
Fixes #2814 by @sarayourfriend
Description
manage.py runserver
forgunicorn --reload
in docker-composeTesting Instructions
Update
api/.env
to haveENVIRONMENT=local
or recreateapi/.env
(rm api/.env && cp api/env.template api/.env
). Then runjust dc stop web && just dc stop nginx && just build web && just api/up
and confirm the API responds and static files work (visit/admin
and confirm the page has styles). Confirm the Nginx image still has static files by visiting the Nginx service locally athttp://0.0.0.0:50270/admin
. This proves the static files are still correctly copied over from the Django image.Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin