feat: Allow disable IAM registration #7910
Conversation
|
Warning Rate Limit Exceeded@jackylamhk has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 45 minutes and 15 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThese changes introduce a new feature that conditionally enables user registration in the CVAT application based on the Changes
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 Configration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- cvat/apps/iam/urls.py (2 hunks)
- cvat/settings/base.py (1 hunks)
- docker-compose.yml (1 hunks)
- helm-chart/templates/cvat_backend/server/deployment.yml (1 hunks)
- helm-chart/values.yaml (1 hunks)
Files skipped from review due to trivial changes (2)
- docker-compose.yml
- helm-chart/values.yaml
Additional Context Used
Ruff (4)
cvat/settings/base.py (4)
34-34: Module level import not at top of file
36-36: Module level import not at top of file
80-80:
keys.secret_key.SECRET_KEYimported but unused
723-723: Module level import not at top of file
Additional comments not posted (3)
cvat/apps/iam/urls.py (1)
39-42: Conditional inclusion of registration URL looks correct.Verification successful
The
IAM_REGISTRATION_ENABLEDsetting is correctly defined incvat/settings/base.pyand used incvat/apps/iam/urls.pyto conditionally include the registration URL. This confirms that the setting is correctly propagated and affects the URL patterns as expected.
cvat/settings/base.py: IAM_REGISTRATION_ENABLED = to_bool(os.getenv('IAM_REGISTRATION_ENABLED', False))cvat/settings/base.py: if IAM_REGISTRATION_ENABLED:cvat/apps/iam/urls.py: if settings.IAM_REGISTRATION_ENABLED:Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `IAM_REGISTRATION_ENABLED` setting is correctly propagated and affects the URL patterns as expected. # Test: Check if the setting is correctly read from the environment and affects the urlpatterns. rg --type python $'IAM_REGISTRATION_ENABLED' --files-with-matches | xargs cat | grep 'urlpatterns'Length of output: 126
Script:
#!/bin/bash # Description: Verify that the `IAM_REGISTRATION_ENABLED` setting is correctly propagated and affects the URL patterns as expected. # Test: Check if the setting is correctly read from the environment and affects the urlpatterns. rg --type py IAM_REGISTRATION_ENABLED --files-with-matches | xargs cat | grep 'urlpatterns'Length of output: 233
Script:
#!/bin/bash # Description: Verify that the `IAM_REGISTRATION_ENABLED` setting is correctly propagated and affects the URL patterns as expected. # Test: Locate the specific usage of `IAM_REGISTRATION_ENABLED` in the codebase to ensure it correctly affects the URL patterns. rg --type py 'IAM_REGISTRATION_ENABLED' | rg 'urlpatterns'Length of output: 56
Script:
#!/bin/bash # Description: Verify that the `IAM_REGISTRATION_ENABLED` setting is correctly propagated and affects the URL patterns as expected. # Test: Locate all occurrences of `IAM_REGISTRATION_ENABLED` in the codebase to manually verify its impact on URL patterns. rg --type py 'IAM_REGISTRATION_ENABLED'Length of output: 257
helm-chart/templates/cvat_backend/server/deployment.yml (1)
61-62: Correct addition ofIAM_REGISTRATION_ENABLEDenvironment variable in Helm chart.Verification successful
--- # Verification of `IAM_REGISTRATION_ENABLED` Environment Variable in Helm Chart The `IAM_REGISTRATION_ENABLED` environment variable is correctly set in the Helm chart template. - Location: `helm-chart/templates/cvat_backend/server/deployment.yml` </details> <details> <summary>Scripts Executed</summary> The following scripts were executed for the analysis: --- Script: ```shell #!/bin/bash # Description: Verify that the `IAM_REGISTRATION_ENABLED` environment variable is correctly set in the deployment and affects the application behavior. # Test: Check if the environment variable is set and propagated to the application. kubectl get deployment cvat-backend-server -o jsonpath='{.spec.template.spec.containers[?(@.name=="cvat-backend")].env[?(@.name=="IAM_REGISTRATION_ENABLED")]}'Length of output: 236
Script:
#!/bin/bash # Description: Verify that the `IAM_REGISTRATION_ENABLED` environment variable is correctly set in the Helm chart templates. # Search for the `IAM_REGISTRATION_ENABLED` environment variable in the Helm chart templates. ast-grep --lang yaml --pattern $'- name: IAM_REGISTRATION_ENABLED\n value: $_' helm-chart/templates/Length of output: 210
Script:
#!/bin/bash # Description: Verify that the `IAM_REGISTRATION_ENABLED` environment variable is correctly set in the Helm chart templates. # Search for the `IAM_REGISTRATION_ENABLED` environment variable in the Helm chart templates. rg 'name: IAM_REGISTRATION_ENABLED' helm-chart/templates/Length of output: 158
cvat/settings/base.py (1)
238-241: Conditional inclusion of 'dj_rest_auth.registration' inINSTALLED_APPSis implemented correctly.Verification successful
The verification confirms that the
IAM_REGISTRATION_ENABLEDsetting correctly affects the inclusion ofdj_rest_auth.registrationinINSTALLED_APPS.
cvat/settings/base.py:
'dj_rest_auth.registration'is conditionally appended toINSTALLED_APPS.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `IAM_REGISTRATION_ENABLED` setting is correctly affecting the Django settings. # Test: Check if the setting is correctly affecting the `INSTALLED_APPS`. rg --type python $'INSTALLED_APPS' --files-with-matches | xargs cat | grep 'dj_rest_auth.registration'Length of output: 130
Script:
#!/bin/bash # Description: Verify that the `IAM_REGISTRATION_ENABLED` setting is correctly affecting the Django settings. # Test: Check if the setting is correctly affecting the `INSTALLED_APPS`. rg 'INSTALLED_APPS' --type py | xargs grep 'dj_rest_auth.registration'Length of output: 924
Script:
#!/bin/bash # Description: Verify that the `IAM_REGISTRATION_ENABLED` setting is correctly affecting the Django settings. # Test: Check if the setting is correctly affecting the `INSTALLED_APPS`. rg 'INSTALLED_APPS' --type py | xargs -I {} grep 'dj_rest_auth.registration' {}Length of output: 613
Script:
#!/bin/bash # Description: Verify that the `IAM_REGISTRATION_ENABLED` setting is correctly affecting the Django settings. # Test: Check if the setting is correctly affecting the `INSTALLED_APPS`. rg 'INSTALLED_APPS' --type py -l | xargs grep 'dj_rest_auth.registration'Length of output: 203
723c426 to
4abe4ae
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- cvat/apps/iam/urls.py (2 hunks)
- cvat/settings/base.py (1 hunks)
- docker-compose.yml (1 hunks)
- helm-chart/templates/cvat_backend/server/deployment.yml (1 hunks)
- helm-chart/values.yaml (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- cvat/apps/iam/urls.py
- docker-compose.yml
- helm-chart/templates/cvat_backend/server/deployment.yml
- helm-chart/values.yaml
Additional Context Used
Ruff (4)
cvat/settings/base.py (4)
34-34: Module level import not at top of file
36-36: Module level import not at top of file
80-80:
keys.secret_key.SECRET_KEYimported but unused
723-723: Module level import not at top of file
cvat/settings/base.py
Outdated
| IAM_REGISTRATION_ENABLED = to_bool(os.getenv('IAM_REGISTRATION_ENABLED', False)) | ||
| if IAM_REGISTRATION_ENABLED: | ||
| INSTALLED_APPS.append('dj_rest_auth.registration') | ||
|
|
There was a problem hiding this comment.
Tip
Codebase Verification
Ensure the conditional inclusion of 'dj_rest_auth.registration' is tested under different environments.
- No tests were found that cover the conditional inclusion of 'dj_rest_auth.registration' or the
IAM_REGISTRATION_ENABLEDsetting. - Recommend adding tests to verify that 'dj_rest_auth.registration' is included in
INSTALLED_APPSonly whenIAM_REGISTRATION_ENABLEDis true.
Analysis chain
Ensure the conditional inclusion of 'dj_rest_auth.registration' is tested under different environments.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the conditional inclusion of 'dj_rest_auth.registration' in INSTALLED_APPS.
# Test: Search for the usage of 'dj_rest_auth.registration' in the codebase. Expect: Only occurances when IAM_REGISTRATION_ENABLED is true.
rg --type python $'dj_rest_auth.registration'
Length of output: 75
Script:
#!/bin/bash
# Description: Verify the conditional inclusion of 'dj_rest_auth.registration' in INSTALLED_APPS.
# Test: Search for the usage of 'dj_rest_auth.registration' in the codebase. Expect: Only occurrences when IAM_REGISTRATION_ENABLED is true.
rg 'dj_rest_auth.registration'
Length of output: 340
Script:
#!/bin/bash
# Description: Verify the existence of tests for the conditional inclusion of 'dj_rest_auth.registration' and related functionality.
# Test: Search for tests related to 'dj_rest_auth.registration' and `IAM_REGISTRATION_ENABLED`.
rg 'dj_rest_auth.registration' --glob '*test*.py'
rg 'IAM_REGISTRATION_ENABLED' --glob '*test*.py'
Length of output: 98
Script:
#!/bin/bash
# Description: Verify the existence of tests for the INSTALLED_APPS configuration and IAM functionality.
# Test: Search for tests related to INSTALLED_APPS configuration.
rg 'INSTALLED_APPS' --glob '*test*.py'
# Test: Search for tests related to IAM functionality.
rg 'IAM' --glob '*test*.py'
Length of output: 121
Consider adding a default value for IAM_REGISTRATION_ENABLED in the environment configuration to ensure consistency across different deployment environments.
- IAM_REGISTRATION_ENABLED = to_bool(os.getenv('IAM_REGISTRATION_ENABLED', False))
+ IAM_REGISTRATION_ENABLED = to_bool(os.getenv('IAM_REGISTRATION_ENABLED', 'False'))Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| IAM_REGISTRATION_ENABLED = to_bool(os.getenv('IAM_REGISTRATION_ENABLED', False)) | |
| if IAM_REGISTRATION_ENABLED: | |
| INSTALLED_APPS.append('dj_rest_auth.registration') | |
| IAM_REGISTRATION_ENABLED = to_bool(os.getenv('IAM_REGISTRATION_ENABLED', 'False')) | |
| if IAM_REGISTRATION_ENABLED: | |
| INSTALLED_APPS.append('dj_rest_auth.registration') | |
Refactor the handling of IAM_REGISTRATION_ENABLED to ensure it is robust against different types of input values.
- IAM_REGISTRATION_ENABLED = to_bool(os.getenv('IAM_REGISTRATION_ENABLED', False))
+ IAM_REGISTRATION_ENABLED = to_bool(os.getenv('IAM_REGISTRATION_ENABLED', 'False').strip().lower() in ['true', '1', 't', 'y', 'yes'])Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| IAM_REGISTRATION_ENABLED = to_bool(os.getenv('IAM_REGISTRATION_ENABLED', False)) | |
| if IAM_REGISTRATION_ENABLED: | |
| INSTALLED_APPS.append('dj_rest_auth.registration') | |
| IAM_REGISTRATION_ENABLED = to_bool(os.getenv('IAM_REGISTRATION_ENABLED', 'False').strip().lower() in ['true', '1', 't', 'y', 'yes']) | |
| if IAM_REGISTRATION_ENABLED: | |
| INSTALLED_APPS.append('dj_rest_auth.registration') | |
d5f4a00 to
9cf452f
Compare
|
I will close PR due to failed tests and no activity for a long time |
Motivation and context
How 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
IAM_REGISTRATION_ENABLEDsetting.Configuration
IAM_REGISTRATION_ENABLEDenvironment variable to control registration feature.docker-composeand Helm chart configurations to includeIAM_REGISTRATION_ENABLEDsetting.