Skip to content
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

Fix pylint_django_settings plugin #35497

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions .github/workflows/pylint-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,19 @@ jobs:
matrix:
include:
- module-name: lms-1
path: "--django-settings-module=lms.envs.test lms/djangoapps/badges/ lms/djangoapps/branding/ lms/djangoapps/bulk_email/ lms/djangoapps/bulk_enroll/ lms/djangoapps/bulk_user_retirement/ lms/djangoapps/ccx/ lms/djangoapps/certificates/ lms/djangoapps/commerce/ lms/djangoapps/course_api/ lms/djangoapps/course_blocks/ lms/djangoapps/course_home_api/ lms/djangoapps/course_wiki/ lms/djangoapps/coursewarehistoryextended/ lms/djangoapps/debug/ lms/djangoapps/courseware/ lms/djangoapps/course_goals/ lms/djangoapps/rss_proxy/"
path: "lms/djangoapps/badges/ lms/djangoapps/branding/ lms/djangoapps/bulk_email/ lms/djangoapps/bulk_enroll/ lms/djangoapps/bulk_user_retirement/ lms/djangoapps/ccx/ lms/djangoapps/certificates/ lms/djangoapps/commerce/ lms/djangoapps/course_api/ lms/djangoapps/course_blocks/ lms/djangoapps/course_home_api/ lms/djangoapps/course_wiki/ lms/djangoapps/coursewarehistoryextended/ lms/djangoapps/debug/ lms/djangoapps/courseware/ lms/djangoapps/course_goals/ lms/djangoapps/rss_proxy/"
- module-name: lms-2
path: "--django-settings-module=lms.envs.test lms/djangoapps/gating/ lms/djangoapps/grades/ lms/djangoapps/instructor/ lms/djangoapps/instructor_analytics/ lms/djangoapps/discussion/ lms/djangoapps/edxnotes/ lms/djangoapps/email_marketing/ lms/djangoapps/experiments/ lms/djangoapps/instructor_task/ lms/djangoapps/learner_dashboard/ lms/djangoapps/learner_home/ lms/djangoapps/lms_initialization/ lms/djangoapps/lms_xblock/ lms/djangoapps/lti_provider/ lms/djangoapps/mailing/ lms/djangoapps/mobile_api/ lms/djangoapps/monitoring/ lms/djangoapps/ora_staff_grader/ lms/djangoapps/program_enrollments/ lms/djangoapps/rss_proxy lms/djangoapps/static_template_view/ lms/djangoapps/staticbook/ lms/djangoapps/support/ lms/djangoapps/survey/ lms/djangoapps/teams/ lms/djangoapps/tests/ lms/djangoapps/user_tours/ lms/djangoapps/verify_student/ lms/djangoapps/mfe_config_api/ lms/envs/ lms/lib/ lms/tests.py"
path: "lms/djangoapps/gating/ lms/djangoapps/grades/ lms/djangoapps/instructor/ lms/djangoapps/instructor_analytics/ lms/djangoapps/discussion/ lms/djangoapps/edxnotes/ lms/djangoapps/email_marketing/ lms/djangoapps/experiments/ lms/djangoapps/instructor_task/ lms/djangoapps/learner_dashboard/ lms/djangoapps/learner_home/ lms/djangoapps/lms_initialization/ lms/djangoapps/lms_xblock/ lms/djangoapps/lti_provider/ lms/djangoapps/mailing/ lms/djangoapps/mobile_api/ lms/djangoapps/monitoring/ lms/djangoapps/ora_staff_grader/ lms/djangoapps/program_enrollments/ lms/djangoapps/rss_proxy lms/djangoapps/static_template_view/ lms/djangoapps/staticbook/ lms/djangoapps/support/ lms/djangoapps/survey/ lms/djangoapps/teams/ lms/djangoapps/tests/ lms/djangoapps/user_tours/ lms/djangoapps/verify_student/ lms/djangoapps/mfe_config_api/ lms/envs/ lms/lib/ lms/tests.py"
- module-name: openedx-1
path: "--django-settings-module=lms.envs.test openedx/core/types/ openedx/core/djangoapps/ace_common/ openedx/core/djangoapps/agreements/ openedx/core/djangoapps/api_admin/ openedx/core/djangoapps/auth_exchange/ openedx/core/djangoapps/bookmarks/ openedx/core/djangoapps/cache_toolbox/ openedx/core/djangoapps/catalog/ openedx/core/djangoapps/ccxcon/ openedx/core/djangoapps/commerce/ openedx/core/djangoapps/common_initialization/ openedx/core/djangoapps/common_views/ openedx/core/djangoapps/config_model_utils/ openedx/core/djangoapps/content/ openedx/core/djangoapps/content_libraries/ openedx/core/djangoapps/content_staging/ openedx/core/djangoapps/contentserver/ openedx/core/djangoapps/cookie_metadata/ openedx/core/djangoapps/cors_csrf/ openedx/core/djangoapps/course_apps/ openedx/core/djangoapps/course_date_signals/ openedx/core/djangoapps/course_groups/ openedx/core/djangoapps/courseware_api/ openedx/core/djangoapps/crawlers/ openedx/core/djangoapps/credentials/ openedx/core/djangoapps/credit/ openedx/core/djangoapps/dark_lang/ openedx/core/djangoapps/debug/ openedx/core/djangoapps/discussions/ openedx/core/djangoapps/django_comment_common/ openedx/core/djangoapps/embargo/ openedx/core/djangoapps/enrollments/ openedx/core/djangoapps/external_user_ids/ openedx/core/djangoapps/zendesk_proxy/ openedx/core/djangolib/ openedx/core/lib/ openedx/core/tests/ openedx/core/djangoapps/course_live/"
path: "openedx/core/types/ openedx/core/djangoapps/ace_common/ openedx/core/djangoapps/agreements/ openedx/core/djangoapps/api_admin/ openedx/core/djangoapps/auth_exchange/ openedx/core/djangoapps/bookmarks/ openedx/core/djangoapps/cache_toolbox/ openedx/core/djangoapps/catalog/ openedx/core/djangoapps/ccxcon/ openedx/core/djangoapps/commerce/ openedx/core/djangoapps/common_initialization/ openedx/core/djangoapps/common_views/ openedx/core/djangoapps/config_model_utils/ openedx/core/djangoapps/content/ openedx/core/djangoapps/content_libraries/ openedx/core/djangoapps/content_staging/ openedx/core/djangoapps/contentserver/ openedx/core/djangoapps/cookie_metadata/ openedx/core/djangoapps/cors_csrf/ openedx/core/djangoapps/course_apps/ openedx/core/djangoapps/course_date_signals/ openedx/core/djangoapps/course_groups/ openedx/core/djangoapps/courseware_api/ openedx/core/djangoapps/crawlers/ openedx/core/djangoapps/credentials/ openedx/core/djangoapps/credit/ openedx/core/djangoapps/dark_lang/ openedx/core/djangoapps/debug/ openedx/core/djangoapps/discussions/ openedx/core/djangoapps/django_comment_common/ openedx/core/djangoapps/embargo/ openedx/core/djangoapps/enrollments/ openedx/core/djangoapps/external_user_ids/ openedx/core/djangoapps/zendesk_proxy/ openedx/core/djangolib/ openedx/core/lib/ openedx/core/tests/ openedx/core/djangoapps/course_live/"
- module-name: openedx-2
path: "--django-settings-module=lms.envs.test openedx/core/djangoapps/geoinfo/ openedx/core/djangoapps/header_control/ openedx/core/djangoapps/heartbeat/ openedx/core/djangoapps/lang_pref/ openedx/core/djangoapps/models/ openedx/core/djangoapps/monkey_patch/ openedx/core/djangoapps/oauth_dispatch/ openedx/core/djangoapps/olx_rest_api/ openedx/core/djangoapps/password_policy/ openedx/core/djangoapps/plugin_api/ openedx/core/djangoapps/plugins/ openedx/core/djangoapps/profile_images/ openedx/core/djangoapps/programs/ openedx/core/djangoapps/safe_sessions/ openedx/core/djangoapps/schedules/ openedx/core/djangoapps/service_status/ openedx/core/djangoapps/session_inactivity_timeout/ openedx/core/djangoapps/signals/ openedx/core/djangoapps/site_configuration/ openedx/core/djangoapps/system_wide_roles/ openedx/core/djangoapps/theming/ openedx/core/djangoapps/user_api/ openedx/core/djangoapps/user_authn/ openedx/core/djangoapps/util/ openedx/core/djangoapps/verified_track_content/ openedx/core/djangoapps/video_config/ openedx/core/djangoapps/video_pipeline/ openedx/core/djangoapps/waffle_utils/ openedx/core/djangoapps/xblock/ openedx/core/djangoapps/xmodule_django/ openedx/core/tests/ openedx/features/ openedx/testing/ openedx/tests/ openedx/core/djangoapps/notifications/ openedx/core/djangoapps/staticfiles/ openedx/core/djangoapps/content_tagging/"
path: "openedx/core/djangoapps/geoinfo/ openedx/core/djangoapps/header_control/ openedx/core/djangoapps/heartbeat/ openedx/core/djangoapps/lang_pref/ openedx/core/djangoapps/models/ openedx/core/djangoapps/monkey_patch/ openedx/core/djangoapps/oauth_dispatch/ openedx/core/djangoapps/olx_rest_api/ openedx/core/djangoapps/password_policy/ openedx/core/djangoapps/plugin_api/ openedx/core/djangoapps/plugins/ openedx/core/djangoapps/profile_images/ openedx/core/djangoapps/programs/ openedx/core/djangoapps/safe_sessions/ openedx/core/djangoapps/schedules/ openedx/core/djangoapps/service_status/ openedx/core/djangoapps/session_inactivity_timeout/ openedx/core/djangoapps/signals/ openedx/core/djangoapps/site_configuration/ openedx/core/djangoapps/system_wide_roles/ openedx/core/djangoapps/theming/ openedx/core/djangoapps/user_api/ openedx/core/djangoapps/user_authn/ openedx/core/djangoapps/util/ openedx/core/djangoapps/verified_track_content/ openedx/core/djangoapps/video_config/ openedx/core/djangoapps/video_pipeline/ openedx/core/djangoapps/waffle_utils/ openedx/core/djangoapps/xblock/ openedx/core/djangoapps/xmodule_django/ openedx/core/tests/ openedx/features/ openedx/testing/ openedx/tests/ openedx/core/djangoapps/notifications/ openedx/core/djangoapps/staticfiles/ openedx/core/djangoapps/content_tagging/"
- module-name: common
path: "--django-settings-module=lms.envs.test common pavelib"
path: "common pavelib"
- module-name: cms
path: "--django-settings-module=cms.envs.test cms"
path: "cms"
- module-name: xmodule
path: "--django-settings-module=lms.envs.test xmodule"
path: "xmodule"

name: pylint ${{ matrix.module-name }}
steps:
Expand Down
83 changes: 38 additions & 45 deletions pylint_django_settings.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
from pylint.interfaces import IAstroidChecker
from pylint.checkers import BaseChecker
from pylint_django.checkers import ForeignKeyStringsChecker
from pylint_plugin_utils import get_checker

Expand All @@ -8,52 +6,47 @@ class ArgumentCompatibilityError(Exception):
pass


class SetDjangoSettingsChecker(BaseChecker):
def _get_django_settings_module(arguments):
"""
This isn't a checker, but setting django settings module when pylint command is ran.
This is to avoid 'django-not-configured' pylint warning
Determines the appropriate Django settings module based on the pylint command-line arguments.
It prevents the use of cms modules alongside lms or common modules.
:param arguments: List of command-line arguments passed to pylint
:return: A string representing the correct Django settings module ('cms.envs.test' or 'lms.envs.test')
:raises ArgumentCompatibilityError: If both cms and lms/common modules are present
"""
__implements__ = IAstroidChecker

name = 'set-django-settings'

msgs = {'R0991': ('bogus', 'bogus', 'bogus')}

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

def open(self):
name_checker = get_checker(self.linter, ForeignKeyStringsChecker)
# pylint command should not run with modules from both cms and (lms, common) at once
cms_module = False
lms_module = False
common_module = False
arguments = self.linter.cmdline_parser.parse_args()[1]
for arg in arguments:
if arg.startswith('cms'):
cms_module = True
elif arg.startswith('lms'):
lms_module = True
elif arg.startswith('common'):
common_module = True

if cms_module and (lms_module or common_module):
# when cms module is present in pylint command, it can't be parired with (lms, common)
# as common and lms gives error with cms test settings
raise ArgumentCompatibilityError(
"Modules from both common and lms can't be paired with cms while running pylint"
)
elif cms_module:
# If a module from cms is present in pylint command arguments
# and ony other module from (openedx, pavelib) is present
# than test setting of cms is used
name_checker.config.django_settings_module = 'cms.envs.test'
else:
# If any module form (lms, common, openedx, pavelib) is present in
# pylint command arguments than test setting of lms is used
name_checker.config.django_settings_module = 'lms.envs.test'
cms_module, lms_module, common_module = False, False, False

for arg in arguments:
if arg.startswith("cms"):
cms_module = True
elif arg.startswith("lms"):
lms_module = True
elif arg.startswith("common"):
common_module = True

if cms_module and (lms_module or common_module):
# when cms module is present in pylint command, it can't be parired with (lms, common)
# as common and lms gives error with cms test settings
raise ArgumentCompatibilityError(
"Modules from both common and lms cannot be paired with cms when running pylint"
)

# Return the appropriate Django settings module based on the arguments
return "cms.envs.test" if cms_module else "lms.envs.test"


def register(linter):
linter.register_checker(SetDjangoSettingsChecker(linter))
"""
Placeholder function to register the plugin with pylint.
UsamaSadiq marked this conversation as resolved.
Show resolved Hide resolved
"""
pass


def load_configuration(linter):
"""
Configures the Django settings module based on the command-line arguments passed to pylint.
"""
name_checker = get_checker(linter, ForeignKeyStringsChecker)
arguments = linter.cmdline_parser.parse_args()[1]
name_checker.config.django_settings_module = _get_django_settings_module(arguments)
9 changes: 5 additions & 4 deletions pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,12 @@
# SERIOUSLY.
#
# ------------------------------
# Generated by edx-lint version: 5.3.0
# Generated by edx-lint version: 5.3.7
# ------------------------------
[MASTER]
ignore = ,.git,.tox,migrations,node_modules,.pycharm_helpers
persistent = yes
load-plugins = edx_lint.pylint,pylint_django,pylint_celery,pylint_pytest
load-plugins = edx_lint.pylint,pylint_django_settings,pylint_django,pylint_celery,pylint_pytest

[MESSAGES CONTROL]
enable =
Expand Down Expand Up @@ -259,6 +259,7 @@ enable =
useless-suppression,
disable =
bad-indentation,
broad-exception-raised,
consider-using-f-string,
duplicate-code,
file-ignored,
Expand Down Expand Up @@ -407,6 +408,6 @@ ext-import-graph =
int-import-graph =

[EXCEPTIONS]
overgeneral-exceptions = Exception
overgeneral-exceptions = builtins.Exception

# 567bf30b121db79bc07a7028651f7efa0724e8a4
# e624ea03d8124aa9cf2e577f830632344a0a07d9
2 changes: 1 addition & 1 deletion pylintrc_tweaks
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# pylintrc tweaks for use with edx_lint.
[MASTER]
ignore+ = ,.git,.tox,migrations,node_modules,.pycharm_helpers
load-plugins = edx_lint.pylint,pylint_django,pylint_celery,pylint_pytest
load-plugins = edx_lint.pylint,pylint_django_settings,pylint_django,pylint_celery,pylint_pytest

[MESSAGES CONTROL]
disable+ =
Expand Down
Loading