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

Switch local API dev server to use gunicorn --reload instead of Django's runserver #2814

Closed
sarayourfriend opened this issue Aug 10, 2023 · 4 comments · Fixed by #2936
Closed
Labels
💻 aspect: code Concerns the software code in the repository 🧰 goal: internal improvement Improvement that benefits maintainers, not users 🟧 priority: high Stalls work on the project or its dependents 🧱 stack: api Related to the Django API
Milestone

Comments

@sarayourfriend
Copy link
Collaborator

Problem

Stems from #2566

runserver does not support critical features we will need after converting the app to ASGI:

  1. Actually running the ASGI app in full asynchrony
  2. Multiple workers (to test production configuration)

We're also making the configuration slightly more complex by moving to ASGI, relying on gunicorn and uvicorn workers.

Description

To anticipate these complexities and make the local development environment (a) behave more like production and (b) require fewer changes when switching to full ASGI, swap out manage.py runserver for gunicorn --reload. Once we switch to ASGI in #2790, we'll either continue using gunicorn for development (with multiple workers) or can switch to uvicorn --reload.

runserver handles static file serving locally, while gunicorn and uvicorn won't know what to do. In production we rely on nginx to handle this. We could set up Nginx locally to front the API service, but to keep this simple, it's easier to conditionally add the static route handler that runserver uses to the API's urls configuration. See these changes as an example (which require supporting changes for the ENVIRONMENT variable and changes to the static file configuration generally, all present in that commit).

Additional context

Something along these lines will be necessary for ASGI conversion anyway. Doing this as a separate issue again reduces

@sarayourfriend sarayourfriend added 🟧 priority: high Stalls work on the project or its dependents 💻 aspect: code Concerns the software code in the repository 🧰 goal: internal improvement Improvement that benefits maintainers, not users 🧱 stack: api Related to the Django API labels Aug 10, 2023
@sarayourfriend sarayourfriend added this to the Django ASGI milestone Aug 10, 2023
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Openverse Backlog Aug 10, 2023
@sarayourfriend sarayourfriend moved this from 📋 Backlog to 📅 To do in Openverse Backlog Aug 24, 2023
@ashiramin
Copy link
Contributor

@sarayourfriend I'd like to take a look at this. How high of a priority is it? I don't want be a blocker

@sarayourfriend
Copy link
Collaborator Author

This won't necessarily block anything for another two weeks or so. If that works for you, let me know and I'll assign it to you 🙂

@ashiramin
Copy link
Contributor

ashiramin commented Aug 31, 2023

I just saw this :( I'll start on it soon. Please don't assign me the ticket. I'll take a stab at it and see if I can make progress by early next week. I don't want to block anyone else from picking this up. I'll report back here by Monday next week

@sarayourfriend
Copy link
Collaborator Author

No worries, sounds good, and thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💻 aspect: code Concerns the software code in the repository 🧰 goal: internal improvement Improvement that benefits maintainers, not users 🟧 priority: high Stalls work on the project or its dependents 🧱 stack: api Related to the Django API
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants