-
Notifications
You must be signed in to change notification settings - Fork 3k
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
CI: Add a step to verify that no migrations are missing #8267
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe recent changes enhance the GitHub Actions workflows by streamlining Docker commands and adding a new verification step for database migrations. These updates improve schema integrity and ensure that the database is aligned with application models. Additionally, modifications to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- .github/workflows/full.yml (1 hunks)
- .github/workflows/main.yml (1 hunks)
- cvat/apps/engine/models.py (1 hunks)
Additional comments not posted (5)
.github/workflows/full.yml (2)
148-151
: LGTM!The code changes are approved. The Docker command for verifying migrations is correctly formatted.
136-143
: Ensure the Docker image tag is correct.The Docker image tag used in the
docker run
command should be verified to ensure it matches the tag used during the build step.Verification successful
The Docker image tag is correct.
The
CVAT_VERSION
variable is set to "local" and is used consistently in both the build step and thedocker run
command to verify the API schema.
.github/workflows/full.yml
:CVAT_VERSION
is set to "local".Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the Docker image tag used in the `docker run` command matches the tag used during the build step. # Test: Search for the build step and extract the tag. Expect: The tag should match the tag used in the `docker run` command. rg --type yaml -A 5 'CVAT server. Build and push' .github/workflows/full.ymlLength of output: 284
Script:
#!/bin/bash # Description: Extract the Docker image tag used in the build step within the .github/workflows/full.yml file. # Search for the definition of the CVAT_VERSION variable or any tag-related information. rg --type yaml -A 5 'CVAT_VERSION' .github/workflows/full.ymlLength of output: 1430
.github/workflows/main.yml (2)
110-113
: LGTM!The code changes are approved. The Docker command for verifying migrations is correctly formatted.
98-105
: Ensure the Docker image tag is correct.The Docker image tag used in the
docker run
command should be verified to ensure it matches the tag used during the build step.Verification successful
The Docker image tag is correct.
The Docker image tag
cvat/server
used in thedocker run
command matches the tag used during the build step.
- Build step tag:
cvat/server
- Run command tag:
cvat/server
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the Docker image tag used in the `docker run` command matches the tag used during the build step. # Test: Search for the build step and extract the tag. Expect: The tag should match the tag used in the `docker run` command. rg --type yaml -A 5 'CVAT server. Build and push' .github/workflows/main.ymlLength of output: 311
Script:
#!/bin/bash # Description: Search for the Docker image tag used in the build step within the .github/workflows/main.yml file. # Test: Extract the Docker image tag used during the build step. rg --type yaml -A 10 'docker/build-push-action@v3' .github/workflows/main.ymlLength of output: 836
cvat/apps/engine/models.py (1)
220-220
: Ensure data migration for existing records.Changing
null=True
tonull=False
requires a data migration to ensure existing records comply with the new constraint.
31ec494
to
536a327
Compare
Quality Gate passedIssues Measures |
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.
It's a nice addition, I also wanted to create such a check after several similar incidents.
Motivation and context
Apparently, I forgot to add a migration in #8144 (see #8162). This will ensure that such goofs no longer happen.
How has this been tested?
I've tested it by adding a deliberately broken change to this PR.
Checklist
develop
branch[ ] I have created a changelog fragment[ ] I have updated the documentation accordingly[ ] I have added tests to cover my changes[ ] I have increased versions of npm packages if it is necessary(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
chunk_size
value in data models, improving data integrity.