Job validations (public part) #8321
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 permission handling and quality control features within the application. Key modifications include simplifying the permission generation process by removing unnecessary context retrieval steps, enriching models with new fields for improved quality metrics, and updating serializers for better data validation. These improvements streamline workflows, enhance functionality, and ensure more accurate reporting and permissions management. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Webhook
participant Permissions
participant QualityControl
User->>Webhook: Request project access
Webhook->>Permissions: create_scope_view(request, project_id)
Permissions-->>Webhook: Return permissions
Webhook-->>User: Permissions granted
User->>QualityControl: Submit quality report
QualityControl->>QualityControl: Validate report
QualityControl-->>User: Report saved with metrics
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
cvat/apps/quality_control/serializers.py (1)
103-110: Ensure help texts are clear and accurate.The help texts for the new fields provide guidance on their usage. Ensure that these descriptions are clear and align with the application's functionality.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- cvat/apps/analytics_report/permissions.py (2 hunks)
- cvat/apps/engine/permissions.py (3 hunks)
- cvat/apps/quality_control/migrations/0003_qualityreport_assignee_last_updated_and_more.py (1 hunks)
- cvat/apps/quality_control/models.py (3 hunks)
- cvat/apps/quality_control/quality_reports.py (4 hunks)
- cvat/apps/quality_control/serializers.py (4 hunks)
- cvat/apps/webhooks/permissions.py (1 hunks)
Additional comments not posted (18)
cvat/apps/quality_control/migrations/0003_qualityreport_assignee_last_updated_and_more.py (4)
14-18: Ensureassignee_last_updatedfield is initialized correctly.The
assignee_last_updatedfield is added as aDateTimeFieldwithnull=True. Ensure that this field is correctly initialized in existing records to avoid potential null-related issues.
24-35: Validate choices fortarget_metric.The
target_metricfield uses choices fromQualityTargetMetricType. Ensure these choices are correctly utilized in the application logic.
19-23: Reviewmax_validations_per_jobdefault value.The
max_validations_per_jobfield is added with a default value of 0. Ensure this aligns with the intended logic for job validations.Verification successful
Default Value of
max_validations_per_jobis AppropriateThe default value of 0 for the
max_validations_per_jobfield is consistent with the intended logic. It aligns with the validation logic in the serializers, ensuring the field is non-negative and properly described as limiting validation attempts. This setup appears to be intentional and correctly implemented.
cvat/apps/quality_control/serializers.py: Validation ensuresmax_validations_per_jobis non-negative.cvat/apps/quality_control/models.py: Default value set to 0.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `max_validations_per_job` in the codebase. # Test: Search for the usage of `max_validations_per_job`. Expect: Proper handling of the default value. rg --type python 'max_validations_per_job'Length of output: 73
Script:
#!/bin/bash # Description: Verify the usage of `max_validations_per_job` in the codebase. # Test: Search for the usage of `max_validations_per_job`. Expect: Proper handling of the default value. rg 'max_validations_per_job' --glob '*.py' -A 3Length of output: 2003
37-41: Checktarget_metric_thresholddefault value.The
target_metric_thresholdfield is added with a default value of 0.7. Ensure this default aligns with the quality control requirements.cvat/apps/engine/permissions.py (2)
308-318: LGTM! But verify the function usage in the codebase.The changes enhance the method's flexibility and robustness.
However, ensure that all function calls to
create_scope_viewinProjectPermissionmatch the new signature.Verification successful
All usages of
create_scope_viewmatch the new signature.The function calls in the following files are consistent with the updated method signature:
cvat/apps/webhooks/permissions.pycvat/apps/engine/permissions.pycvat/apps/analytics_report/permissions.pyNo issues found.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `create_scope_view` in `ProjectPermission` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type python -A 5 "ProjectPermission.create_scope_view"Length of output: 90
Script:
#!/bin/bash # Description: Verify all function calls to `create_scope_view` in `ProjectPermission` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type py -A 5 "ProjectPermission.create_scope_view"Length of output: 1594
692-702: LGTM! But verify the function usage in the codebase.The changes enhance the method's flexibility and robustness.
However, ensure that all function calls to
create_scope_viewinJobPermissionmatch the new signature.Verification successful
Function usage matches the new signature.
The
create_scope_viewmethod inJobPermissionis correctly used with the new signature in the codebase.
cvat/apps/analytics_report/permissions.py: Correct usage observed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `create_scope_view` in `JobPermission` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type python -A 5 "JobPermission.create_scope_view"Length of output: 86
Script:
#!/bin/bash # Description: Verify all function calls to `create_scope_view` in `JobPermission` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type py -A 5 "JobPermission.create_scope_view"Length of output: 1199
cvat/apps/quality_control/quality_reports.py (2)
Line range hint
2305-2318: Approved: Addition ofassignee_last_updated.The inclusion of the
assignee_last_updatedparameter enhances the tracking of report updates and is well-integrated into the method.
Line range hint
2425-2438: Approved: Addition ofassignee_last_updated.The inclusion of the
assignee_last_updatedparameter enhances the data stored in reports and is well-integrated into the method.cvat/apps/analytics_report/permissions.py (3)
37-37: Ensure correct handling ofjob_idparameter.The
job_idis retrieved from query parameters or request data, depending on the action. Verify that both sources are correctly handled throughout the application.Verification successful
The
job_idparameter is correctly handled. The retrieval and use ofjob_idfrom both query parameters and request data are implemented as expected in thecvat/apps/analytics_report/permissions.pyfile. No issues were found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify handling of `job_id` in the application. # Test: Search for occurrences of `job_id` retrieval. Expect: Correct handling in both query parameters and request data. rg --type python -A 3 $'job_id = request.query_params.get("job_id")|job_id = request.data.get("job_id")'Length of output: 136
Script:
#!/bin/bash # Description: Verify handling of `job_id` in the application. # Test: Search for occurrences of `job_id` retrieval. Expect: Correct handling in both query parameters and request data. rg -t py -A 3 'job_id = request.query_params.get("job_id")|job_id = request.data.get("job_id")'Length of output: 97
Script:
#!/bin/bash # Description: Broaden the search for `job_id` to understand its usage and retrieval in the codebase. # Test: Search for any occurrences of `job_id` to see how it is being used and retrieved. rg -t py -A 3 'job_id'Length of output: 101647
53-53: Confirmproject_idhandling consistency.Ensure that
project_idis consistently handled across the application, especially in permissions logic.Verification successful
project_idHandling is Consistent Across the ApplicationThe handling of
project_idappears to be consistent across the application, especially within the permissions logic. The parameter is retrieved and used in a similar manner in various parts of the codebase, indicating a standardized approach. No inconsistencies were found in its handling.
project_idis retrieved from bothrequest.query_paramsandrequest.data.- It is used in various contexts, such as function calls and validations, across different files.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify handling of `project_id` in the application. # Test: Search for occurrences of `project_id` retrieval and usage. Expect: Consistent logic across the application. rg --type python -A 3 $'project_id = request.query_params.get("project_id")|project_id = request.data.get("project_id")'Length of output: 152
Script:
#!/bin/bash # Description: Verify handling of `project_id` in the application. # Test: Search for occurrences of `project_id` retrieval and usage. Expect: Consistent logic across the application. rg -A 3 $'project_id = request.query_params.get("project_id")|project_id = request.data.get("project_id")'Length of output: 107
Script:
#!/bin/bash # Description: Broaden the search for `project_id` handling in the application. # Test: Search for occurrences of `project_id` being assigned or used in any context. rg -A 3 'project_id\s*='Length of output: 48208
49-49: Verify the integrity oftask_idretrieval.Ensure that
task_idis correctly derived fromjob.segment.task.idand that this logic is consistent with the rest of the application.Verification successful
Integrity of
task_idretrieval is consistent withinpermissions.pyThe retrieval of
task_idfromjob.segment.task.idis consistent within thecvat/apps/analytics_report/permissions.pyfile. The logic checks iftask_idis truthy before using it, ensuring its integrity in this context. If further verification is needed across the application, manual inspection or additional searches may be required.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify retrieval and usage of `task_id` in the application. # Test: Search for occurrences of `task_id` retrieval from `job.segment.task.id`. Expect: Consistent logic across the application. rg --type python -A 3 $'task_id = job.segment.task.id'Length of output: 86
Script:
#!/bin/bash # Description: Verify retrieval and usage of `task_id` in the application. # Corrected Test: Search for occurrences of `task_id` retrieval from `job.segment.task.id`. Expect: Consistent logic across the application. rg 'task_id = job.segment.task.id' --glob '*.py' -A 3Length of output: 389
cvat/apps/webhooks/permissions.py (1)
42-42: Verifycreate_scope_viewmethod update.Ensure that the
create_scope_viewmethod inProjectPermissioncorrectly handles the newly addedrequestparameter and that this change is reflected in all relevant parts of the application.cvat/apps/quality_control/serializers.py (3)
42-44: New metrics fields added toQualityReportSummarySerializer.The addition of
accuracy,precision, andrecallfields enhances the quality assessment capabilities. Ensure that these fields are correctly populated and used in the application.
79-81: New fields added toQualitySettingsSerializer.The fields
target_metric,target_metric_threshold, andmax_validations_per_jobprovide more control over quality settings. Verify that these fields are correctly integrated into the application logic.
161-163: Validation logic formax_validations_per_job.The validation logic ensures that
max_validations_per_jobis not negative. This is a good practice to prevent invalid data entry.cvat/apps/quality_control/models.py (3)
72-83: NewQualityTargetMetricTypeenum added.This enum introduces metrics for quality assessment. Ensure that these metrics are used consistently across the application.
105-105: Newassignee_last_updatedfield inQualityReport.This field tracks the last update to the assignee. Verify that this field is updated appropriately in the application logic.
221-229: New fields inQualitySettings.The fields
target_metric,target_metric_threshold, andmax_validations_per_jobenhance the configurability of quality settings. Ensure these fields are integrated into the application's quality control processes.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| try: | ||
| project = Project.objects.get(id=project) | ||
| except Project.DoesNotExist as ex: | ||
| raise ValidationError(str(ex)) |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
| try: | ||
| job = Job.objects.get(id=job) | ||
| except Job.DoesNotExist as ex: | ||
| raise ValidationError(str(ex)) |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #8321 +/- ##
========================================
Coverage 83.57% 83.58%
========================================
Files 393 393
Lines 41578 41594 +16
Branches 3841 3841
========================================
+ Hits 34749 34765 +16
Misses 6829 6829
|
|
<!-- Raise an issue to propose your change (https://github.com/cvat-ai/cvat/issues). It helps to avoid duplication of efforts from multiple independent contributors. Discuss your ideas with maintainers to be sure that changes will be approved and merged. Read the [Contribution guide](https://docs.cvat.ai/docs/contributing/). --> <!-- Provide a general summary of your changes in the Title above --> ### Motivation and context <!-- Why is this change required? What problem does it solve? If it fixes an open issue, please link to the issue here. Describe your changes in detail, add screenshots. --> - Added `max_validations_per_job`, `target_metric`, `target_metric_threshold` fields in quality settings - Added `assignee_last_updated` in quality reports - Added separate `accuracy`, `precision`, and `recall` fields in quality summary - Refactored some IAM code ### How has this been tested? <!-- Please describe in detail how you tested your changes. Include details of your testing environment, and the tests you ran to see how your change affects other areas of the code, etc. --> ### Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply. If an item isn't applicable for some reason, then ~~explicitly strikethrough~~ the whole line. If you don't do that, GitHub will show incorrect progress for the pull request. If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [ ] I submit my changes into the `develop` branch - [ ] I have created a changelog fragment <!-- see top comment in CHANGELOG.md --> - [ ] I have updated the documentation accordingly - [ ] I have added tests to cover my changes - [ ] I have linked related issues (see [GitHub docs]( https://help.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword)) - [ ] I have increased versions of npm packages if it is necessary ([cvat-canvas](https://github.com/cvat-ai/cvat/tree/develop/cvat-canvas#versioning), [cvat-core](https://github.com/cvat-ai/cvat/tree/develop/cvat-core#versioning), [cvat-data](https://github.com/cvat-ai/cvat/tree/develop/cvat-data#versioning) and [cvat-ui](https://github.com/cvat-ai/cvat/tree/develop/cvat-ui#versioning)) ### License - [ ] I submit _my code changes_ under the same [MIT License]( https://github.com/cvat-ai/cvat/blob/develop/LICENSE) that covers the project. Feel free to contact the maintainers if that's a concern. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced permission handling for jobs, tasks, and projects, streamlining the creation process. - Introduced new quality control metrics and settings, enabling more granular tracking and reporting. - **Bug Fixes** - Improved validation logic to enforce constraints on job validation attempts. - **Documentation** - Added descriptive help texts for new fields in quality report serializers, clarifying their purpose. - **Chores** - Updated method signatures to improve flexibility and maintainability across permission handling and reporting functionalities. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Motivation and context
max_validations_per_job,target_metric,target_metric_thresholdfields in quality settingsassignee_last_updatedin quality reportsaccuracy,precision, andrecallfields in quality summaryHow has this been tested?
Checklist
developbranch(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores