Add per-application and inter-dependency model graphs to CI/CD #3257
Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds a CI job that generates per-app and inter-app Django model graphs (SVG) using django-extensions and Graphviz, uploads them as an artifact, and registers Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hii @arkid15r, |
|
Can you resolve the conflicts please? @OM-JADHAV25 |
|
@ahmedxgouda can you please tell about which conflicts ?? I'll resolve it !! |
|
@ahmedxgouda thanks for pointing it out -- conflicts are resolved now !!! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/run-ci-cd.yaml:
- Around line 224-242: The current APPS extraction using "poetry run python
manage.py showmigrations | awk '{print $1}'" is brittle; replace it with a
Python-based listing that imports Django settings and yields only your project
apps (e.g., LOCAL_APPS or INSTALLED_APPS filtered for non-third-party apps) so
APPS is populated reliably; update the step to run a short Python one-liner (via
"poetry run python -c '...'" or manage.py shell) that prints each app name,
assign that output to APPS, and then keep the existing loop that calls "poetry
run python manage.py graph_models" for each app.
In @backend/pyproject.toml:
- Line 53: Move the django-extensions entry in pyproject.toml into the
dependencies list so it sits alphabetically between django-cors-headers and
django-ninja; specifically, locate the line with "django-extensions = \"^4.1\""
and cut/paste it so the dependencies remain sorted alphabetically.
🧹 Nitpick comments (2)
.github/workflows/run-ci-cd.yaml (2)
174-179: Job configuration aligns with requirements.The
continue-on-error: truesetting correctly ensures this job is non-blocking, fulfilling the requirement from issue #724. However, the job currently lacks a trigger condition to run only on model file changes.Consider adding a path filter or making this job conditional to avoid running on every PR/push. Per issue #724, it should trigger on changes to model files:
generate-backend-model-graphs: name: Generate Backend Model Graphs if: | github.event_name == 'workflow_dispatch' || contains(github.event.head_commit.modified, 'models')Alternatively, use a separate workflow with path filters for
backend/**/models/**.
262-266: Consider adding retention policy for artifacts.The artifact upload step works correctly but doesn't specify a retention period. GitHub's default is 90 days, which may be longer than needed for model graph artifacts.
📦 Add retention-days to limit storage
- name: Upload model graphs as artifact uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f with: name: backend-model-graphs path: backend/model-graphs + retention-days: 30
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
.github/workflows/run-ci-cd.yamlbackend/pyproject.tomlbackend/settings/base.py
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-31T05:17:39.659Z
Learnt from: kart-u
Repo: OWASP/Nest PR: 3101
File: backend/apps/common/extensions.py:92-98
Timestamp: 2025-12-31T05:17:39.659Z
Learning: In this codebase, import OperationType for GraphQL operations from the graphql-core package rather than from strawberry. Use 'from graphql import OperationType'. Strawberry re-exports via graphql-core internally, so relying on strawberry's API may be brittle. Apply this rule to all Python files that deal with GraphQL operation types; ensure imports come from graphql (graphql-core) and not from strawberry packages. This improves compatibility and avoids coupling to strawberry's internals.
Applied to files:
backend/settings/base.py
📚 Learning: 2026-01-01T17:48:23.963Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:41-47
Timestamp: 2026-01-01T17:48:23.963Z
Learning: In Django code, be aware that a QuerySet's boolean evaluation (e.g., if not queryset) runs a database query to determine emptiness. While it is technically valid to use the queryset in a boolean context, use queryset.exists() for existence checks to avoid unnecessary queries and improve performance. Applicable broadly to Python/Django files rather than just this specific path.
Applied to files:
backend/settings/base.py
📚 Learning: 2025-12-21T19:03:59.068Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 1139
File: .github/workflows/setup-backend-environment/action.yaml:16-27
Timestamp: 2025-12-21T19:03:59.068Z
Learning: Composite actions (runs: using: composite) execute as steps within the calling job's context and can access the job context, including job.services.* properties (e.g., job.services.<service_id>.id, job.services.<service_id>.ports). Service containers must be defined at the job level, but a composite action's steps can reference them via the job context.
Applied to files:
.github/workflows/run-ci-cd.yaml
🔇 Additional comments (2)
backend/settings/base.py (1)
47-54: LGTM!The
django_extensionsapp is correctly added toTHIRD_PARTY_APPS, which enables thegraph_modelsmanagement command used by the CI job..github/workflows/run-ci-cd.yaml (1)
206-222: Environment variable approach is pragmatic for CI.Setting dummy values for required secrets allows Django to boot without actual credentials. This is acceptable for graph generation since no database/external service connections are needed.
| strawberry-graphql-django = "^0.73.0" | ||
| thefuzz = "^0.22.1" | ||
| pyparsing = "^3.2.3" | ||
| django-extensions = "^4.1" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat backend/pyproject.toml | head -100Repository: OWASP/Nest
Length of output: 3129
🌐 Web query:
django-extensions 4.1 Django 6.0 compatibility
💡 Result:
Short answer: Yes — django-extensions 4.1 declares a requirement of "Django 4.2 or later", so it is expected to be compatible with Django 6.0. [1][2]
Sources:
- PyPI project page (requirements: Django 4.2 or later). [1]
- django-extensions GitHub / release notes (4.1 release). [2]
Move django-extensions to maintain alphabetical ordering of dependencies.
The django-extensions dependency should be placed between django-cors-headers and django-ninja to maintain alphabetical order with other dependencies in this section.
Suggested placement
django-cors-headers = "^4.7.0"
+django-extensions = "^4.1"
django-ninja = "^1.4.3"🤖 Prompt for AI Agents
In @backend/pyproject.toml at line 53, Move the django-extensions entry in
pyproject.toml into the dependencies list so it sits alphabetically between
django-cors-headers and django-ninja; specifically, locate the line with
"django-extensions = \"^4.1\"" and cut/paste it so the dependencies remain
sorted alphabetically.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
.github/workflows/run-ci-cd.yaml (5)
184-198: Consider adding dependency caching for faster CI runs.This job installs Poetry and backend dependencies without caching, unlike other jobs in this workflow (e.g.,
pre-commitat lines 40-45). For consistency and performance:
- Use
pipx install poetry(matching line 38) instead ofpip install poetry.- Add Poetry caching to
actions/setup-python.Suggested improvement
- name: Set up Python uses: actions/setup-python@83679a892e2d95755f2dac6acb0bfd1e9ac5d548 with: + cache: 'poetry' + cache-dependency-path: backend/poetry.lock python-version: '3.13' - name: Install system dependencies run: | sudo apt-get update sudo apt-get install -y graphviz - name: Install backend dependencies working-directory: backend run: | - pip install poetry + pipx install poetry poetry install --no-interaction --no-root
234-238: Redundant condition in app filtering.The check
not a.startswith("django.contrib.")is redundant since anything starting with"django.contrib."already starts with"django.", which is already excluded by the first condition.Simplified filtering
apps = [ a for a in settings.INSTALLED_APPS if not a.startswith("django.") - and not a.startswith("django.contrib.") ]
256-272: Minor: Inconsistent error handling style.This step uses
set +eglobally, making the|| trueon each command redundant. The per-app step usesset -ewith|| trueon each command. Consider using one consistent approach across both steps.Option A: Match per-app step style
- name: Generate inter-app model graphs working-directory: backend run: | - set +e + set -e poetry run python manage.py graph_models \ --all-applications \ --output "model-graphs/inter-app/backend_inheritance.svg" \ --format svg || true poetry run python manage.py graph_models \ --all-applications \ --disable-inheritance \ --output "model-graphs/inter-app/backend_relations.svg" \ --format svg || true - - set -e
274-278: Consider addingif: always()to ensure artifact upload.If an early step fails (e.g., Poetry install), the artifact upload won't run and you'll lose any partial results. Adding
if: always()ensures artifacts are uploaded regardless of earlier step outcomes, which aids debugging.Suggested change
- name: Upload model graphs as artifact + if: always() uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f with: name: backend-model-graphs path: backend/model-graphs
174-179: Job configuration is sound; path-based filtering is optional.The
continue-on-error: truecorrectly implements non-blocking behavior. The job configuration runs on all standard triggers (pull requests, pushes, workflow dispatch) on ubuntu-latest, which is appropriate for model graph generation. If you want to optimize CI time by skipping this job when model files are unchanged, consider adding a conditional withif: contains(github.event.pull_request.files.*.filename, 'models'); however, this requires careful handling depending on your trigger context (note: this works reliably forpull_requestevents but has limitations forpushevents).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/run-ci-cd.yaml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-21T19:03:59.068Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 1139
File: .github/workflows/setup-backend-environment/action.yaml:16-27
Timestamp: 2025-12-21T19:03:59.068Z
Learning: Composite actions (runs: using: composite) execute as steps within the calling job's context and can access the job context, including job.services.* properties (e.g., job.services.<service_id>.id, job.services.<service_id>.ports). Service containers must be defined at the job level, but a composite action's steps can reference them via the job context.
Applied to files:
.github/workflows/run-ci-cd.yaml
🔇 Additional comments (1)
.github/workflows/run-ci-cd.yaml (1)
206-222: Environment configuration looks appropriate.Using dummy/None values for secrets and service credentials is correct here since
graph_modelsonly needs Django to load settings and introspect models without connecting to external services.
2 participants
Proposed change
Resolves #724
This PR adds per-application and inter-dependency model graph generation to CI/CD, with graphs separated by inheritance and relationship views. The workflow is non-blocking for CI and all generated images are stored as CI artifacts for inspection.
Checklist
make check-testlocally and all tests passed