feat/django foundation - #4
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
🟡 Not ready to approve
The new Django foundation introduces security/configuration and maintainability issues (committed SECRET_KEY, missing app registration/ASGI setting, and formatting/noise in the updated health test) that should be corrected before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR migrates the service scaffold from FastAPI to a Django project foundation, adds a basic /health/ endpoint, and updates the test suite and dependency management to align with the new framework.
Changes:
- Replaced the FastAPI app scaffold with a Django project (
config/,manage.py) and acoreapp containing a health check view. - Updated pytest configuration and rewrote the health test to use Django’s URL routing (
reverse("health")). - Switched dependency management from
requirements.txttopyproject.tomldependency groups (withuv.lock).
File summaries
| File | Description |
|---|---|
| uv.lock | Adds a uv lockfile capturing Django/pytest dependencies for the new stack. |
| tests/test_health.py | Rewrites the health test for Django routing/client usage. |
| requirements.txt | Removes the FastAPI-era pinned dependencies. |
| README.md | Removes the FastAPI starter documentation. |
| pytest.ini | Configures pytest for Django via DJANGO_SETTINGS_MODULE. |
| pyproject.toml | Introduces dependency groups for Django + pytest tooling. |
| manage.py | Adds Django’s management entrypoint. |
| LICENSE | Removes the prior Apache 2.0 license file. |
| core/views.py | Adds a health JSON endpoint implementation. |
| core/tests.py | Adds placeholder Django app tests module. |
| core/models.py | Adds placeholder Django app models module. |
| core/migrations/init.py | Initializes Django migrations package for core. |
| core/apps.py | Adds CoreConfig app configuration. |
| core/admin.py | Adds placeholder Django admin registration module. |
| core/init.py | Initializes the core Django app package. |
| config/wsgi.py | Adds WSGI entrypoint for Django deployments. |
| config/urls.py | Defines URL routing including /health/. |
| config/settings.py | Adds baseline Django settings for the new project. |
| config/asgi.py | Adds ASGI entrypoint for Django deployments. |
| config/init.py | Initializes the config Django project package. |
| app/routes/health.py | Removes the FastAPI health router. |
| app/main.py | Removes the FastAPI application entrypoint. |
| app/init.py | Removes the FastAPI APP_VERSION constant. |
| .python-version | Pins the project Python version to 3.12. |
| .gitignore | Rewrites .gitignore content (no functional change apparent). |
| .env.example | Keeps env example but still references FastAPI in comments. |
Review details
- Files reviewed: 19/26 changed files
- Comments generated: 6
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Comment on lines
+33
to
+40
| INSTALLED_APPS = [ | ||
| "django.contrib.admin", | ||
| "django.contrib.auth", | ||
| "django.contrib.contenttypes", | ||
| "django.contrib.sessions", | ||
| "django.contrib.messages", | ||
| "django.contrib.staticfiles", | ||
| ] |
| }, | ||
| ] | ||
|
|
||
| WSGI_APPLICATION = "config.wsgi.application" |
Comment on lines
+1
to
+12
| from django.shortcuts import render | ||
| from django.http import JsonResponse | ||
|
|
||
|
|
||
|
|
||
| def health(request): | ||
| return JsonResponse( | ||
| { | ||
| "status": "ok", | ||
| "service": "curricula.live api", | ||
| } | ||
| ) |
Comment on lines
+1
to
+11
| from django.urls import reverse | ||
|
|
||
| def test_health_endpoint(client): | ||
| # respsonse = client.get("/health/") | ||
| response = client.get(reverse("health")) | ||
|
|
||
| assert response.status_code == 200 | ||
| assert response.json() == { | ||
| "status": "ok", | ||
| "service": "curricula.live api", | ||
| } |
| @@ -1,2 +1,2 @@ | |||
| # FastAPI app settings | |||
| APP_ENV=development | |||
| # FastAPI app settings | |||
Comment on lines
+13
to
+28
| from pathlib import Path | ||
|
|
||
| # Build paths inside the project like this: BASE_DIR / 'subdir'. | ||
| BASE_DIR = Path(__file__).resolve().parent.parent | ||
|
|
||
|
|
||
| # Quick-start development settings - unsuitable for production | ||
| # See https://docs.djangoproject.com/en/5.2/howto/deployment/checklist/ | ||
|
|
||
| # SECURITY WARNING: keep the secret key used in production secret! | ||
| SECRET_KEY = "django-insecure-uxp7)i7y(y6f4u%pra%l#@njpls!^(v*b@e@8m7%5q#25@ll0w" | ||
|
|
||
| # SECURITY WARNING: don't run with debug turned on in production! | ||
| DEBUG = True | ||
|
|
||
| ALLOWED_HOSTS = [] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Describe the change.
Type of change
Checklist
Related repositories / organizations
List related repos or orgs.