-
-
Notifications
You must be signed in to change notification settings - Fork 96
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
Model graph image #1179
base: main
Are you sure you want to change the base?
Model graph image #1179
Conversation
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Summary by CodeRabbitThis release brings a new automated process that generates and keeps our application's data model diagram up-to-date, offering clearer insights into the system structure. Additionally, enhancements to supporting tools and configurations improve the overall reliability of this feature.
WalkthroughThis change introduces a new GitHub Actions workflow that generates and updates a model relationship diagram for the backend. The workflow is triggered by changes in Python model files and can also be manually activated. Additionally, the backend configuration has been updated by adding two new dependencies— Changes
Assessment against linked issues
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Hey @arkid15r |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/generate-erd.yaml (1)
58-64
: Consider storing the graph image in a dedicated documentation directoryCurrently, the image is stored in the backend root directory. Consider storing it in a dedicated docs folder for better organization.
- DJANGO_CONFIGURATION=$DJANGO_CONFIGURATION DJANGO_SETTINGS_MODULE=$DJANGO_SETTINGS_MODULE poetry run python manage.py graph_models -a -o models_relations.png + DJANGO_CONFIGURATION=$DJANGO_CONFIGURATION DJANGO_SETTINGS_MODULE=$DJANGO_SETTINGS_MODULE poetry run python manage.py graph_models -a -o ../docs/models_relations.pngAnd update the related paths:
- path: backend/models_relations.png + path: docs/models_relations.png- git add models_relations.png + git add ../docs/models_relations.png
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
backend/models_relations.png
is excluded by!**/*.png
backend/poetry.lock
is excluded by!**/*.lock
📒 Files selected for processing (3)
.github/workflows/generate-erd.yaml
(1 hunks)backend/pyproject.toml
(1 hunks)backend/settings/local.py
(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
backend/settings/local.py (1)
backend/settings/base.py (1)
Base
(9-199)
🔇 Additional comments (6)
backend/settings/local.py (1)
22-22
: Appropriate addition of django_extensions to local settingsThis correctly adds django_extensions to the INSTALLED_APPS for the local environment only, which is the best practice since it's a development tool rather than a production requirement.
backend/pyproject.toml (1)
49-50
: Appropriate addition of development dependenciesThe additions of django-extensions and pydotplus as development dependencies align with the purpose of generating model graphs. Both are correctly specified with caret version constraints and appropriately placed in the dev dependencies group.
.github/workflows/generate-erd.yaml (4)
1-8
: Well-structured workflow with appropriate triggersThe workflow is properly named and configured with appropriate triggers:
- Triggering on model file changes ensures the graph stays updated
- The manual trigger provides flexibility for on-demand updates
13-32
: Good setup of dependencies and toolsAll necessary dependencies (Poetry, Graphviz, Python) are correctly installed with proper caching. Using Python 3.13 matches the project requirements, and the working directory is correctly set.
34-46
: Proper environment configuration for DjangoThe environment variables are properly set with placeholder values for secrets, which is appropriate since they aren't needed for graph generation.
47-51
: Effective use of django-extensions commandThe command correctly utilizes django-extensions' graph_models management command with appropriate parameters.
655b0e8
to
4954ba9
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
backend/apps/owasp/models/event.py (1)
195-195
: Trailing Newline & Clarify Comment Marker
The new comment#11
appears to serve as a marker but does not alter functionality. If it is meant for debugging or as a placeholder, please consider adding more context or removing it if unnecessary. Additionally, ensure the file ends with a newline to satisfy linting requirements (W292).🧰 Tools
🪛 Ruff (0.8.2)
195-195: No newline at end of file
Add trailing newline
(W292)
.github/workflows/generate-erd.yaml (2)
3-8
: Enhance Workflow Trigger with paths-ignore
To avoid re-triggering the workflow when the generated graph image is committed, consider adding apaths-ignore
field. For example, update the trigger configuration as shown below:-on: - push: - paths: - - "backend/apps/*/models/*.py" - workflow_dispatch: +on: + push: + paths: + - "backend/apps/*/models/*.py" + paths-ignore: + - "backend/models_relations.png" + workflow_dispatch:This change will help prevent unnecessary runs due to automated commits.
20-21
: Update Apt Package List Before Installing Graphviz
It is a best practice to update the package list to ensure the latest packages are available. Consider modifying the "Install Graphviz" step as follows:- - name: Install Graphviz - run: sudo apt-get install -y graphviz + - name: Install Graphviz + run: sudo apt-get update && sudo apt-get install -y graphvizThis reduces the risk of installation failures or outdated package issues.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/models_relations.png
is excluded by!**/*.png
📒 Files selected for processing (2)
.github/workflows/generate-erd.yaml
(1 hunks)backend/apps/owasp/models/event.py
(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
backend/apps/owasp/models/event.py
195-195: No newline at end of file
Add trailing newline
(W292)
e0a5383
to
b5e878c
Compare
b5e878c
to
848f7e7
Compare
Hey @arkid15r |
|
fixes: #724