fix: Fix pylint_django_settings plugin

openedx · Oct 9, 2024 · 6974e9e · 6974e9e
1 parent f5b8839
commit 6974e9e
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 57 deletions.
diff --git a/.github/workflows/pylint-checks.yml b/.github/workflows/pylint-checks.yml
@@ -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:

diff --git a/pylint_django_settings.py b/pylint_django_settings.py
@@ -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
 
@@ -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.
+    """
+    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)
diff --git a/pylintrc b/pylintrc
@@ -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 =
@@ -259,6 +259,7 @@ enable =
 	useless-suppression,
 disable =
 	bad-indentation,
+	broad-exception-raised,
 	consider-using-f-string,
 	duplicate-code,
 	file-ignored,
@@ -407,6 +408,6 @@ ext-import-graph =
 int-import-graph =
 
 [EXCEPTIONS]
-overgeneral-exceptions = Exception
+overgeneral-exceptions = builtins.Exception
 
-# 567bf30b121db79bc07a7028651f7efa0724e8a4
+# e624ea03d8124aa9cf2e577f830632344a0a07d9
diff --git a/pylintrc_tweaks b/pylintrc_tweaks
@@ -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+ =