From 151c4fcfb29473fa6358423d160b0cfc02d2b18f Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Fri, 21 Apr 2023 08:26:48 -0400 Subject: [PATCH] build: move collectstatic ignore patterns into configuration (#31934) Adds a tiny `openedx.core.djangoapps.staticfiles` app so that static asset ignore patterns can be coded into configuration rather than supplied on the command line or coded into pavelib. Makes it easier to run static asset collection without Paver. See ADR for details: openedx/core/djangoapps/staticfiles/docs/decisions/0001-purpose-of-app.rst Closes: https://github.com/openedx/edx-platform/issues/31658 --- .github/workflows/pylint-checks.yml | 2 +- cms/envs/common.py | 6 ++- lms/envs/common.py | 5 ++- .../core/djangoapps/staticfiles/__init__.py | 0 openedx/core/djangoapps/staticfiles/apps.py | 32 ++++++++++++++ .../docs/decisions/0001-purpose-of-app.rst | 43 +++++++++++++++++++ pavelib/assets.py | 23 +--------- 7 files changed, 86 insertions(+), 25 deletions(-) create mode 100644 openedx/core/djangoapps/staticfiles/__init__.py create mode 100644 openedx/core/djangoapps/staticfiles/apps.py create mode 100644 openedx/core/djangoapps/staticfiles/docs/decisions/0001-purpose-of-app.rst diff --git a/.github/workflows/pylint-checks.yml b/.github/workflows/pylint-checks.yml index 1ef2157a08..0f1dbd750d 100644 --- a/.github/workflows/pylint-checks.yml +++ b/.github/workflows/pylint-checks.yml @@ -20,7 +20,7 @@ jobs: - 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/demographics/ 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/learner_pathway/ openedx/core/djangoapps/notifications/" + 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/learner_pathway/ openedx/core/djangoapps/notifications/ openedx/core/djangoapps/staticfiles/" - module-name: common path: "--django-settings-module=lms.envs.test common" - module-name: cms diff --git a/cms/envs/common.py b/cms/envs/common.py index 6c73217186..4b72820594 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -1565,8 +1565,12 @@ INSTALLED_APPS = [ 'django.contrib.sessions', 'django.contrib.sites', 'django.contrib.messages', - 'django.contrib.staticfiles', + + # Tweaked version of django.contrib.staticfiles + 'openedx.core.djangoapps.staticfiles.apps.EdxPlatformStaticFilesConfig', + 'django_celery_results', + 'method_override', # Common Initialization diff --git a/lms/envs/common.py b/lms/envs/common.py index bac83c7860..e3e486108f 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2955,7 +2955,10 @@ INSTALLED_APPS = [ 'django.contrib.redirects', 'django.contrib.sessions', 'django.contrib.sites', - 'django.contrib.staticfiles', + + # Tweaked version of django.contrib.staticfiles + 'openedx.core.djangoapps.staticfiles.apps.EdxPlatformStaticFilesConfig', + 'django_celery_results', # Common Initialization diff --git a/openedx/core/djangoapps/staticfiles/__init__.py b/openedx/core/djangoapps/staticfiles/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/staticfiles/apps.py b/openedx/core/djangoapps/staticfiles/apps.py new file mode 100644 index 0000000000..3b86579735 --- /dev/null +++ b/openedx/core/djangoapps/staticfiles/apps.py @@ -0,0 +1,32 @@ +""" +Configuration for static assets. Shared by LMS and CMS. +""" +from django.contrib.staticfiles.apps import StaticFilesConfig + + +class EdxPlatformStaticFilesConfig(StaticFilesConfig): + """ + A thin wrapper around the standard django.contrib.staticfiles app + which adds the proper file & folder ignore patterns for edx-platform + static asset collection. + + This allows devs & operators to run: + ./manage.py [lms|cms] collectstatic + + instead of: + ./manage.py [lms|cms] collectstatic --ignore geoip --ignore sass ... etc. + """ + + ignore_patterns = StaticFilesConfig.ignore_patterns + [ + + "geoip", # Geo-IP data, only accessed in Python + "sass", # We compile these out, don't need the source files in staticfiles + "xmodule_js", # Symlink for tests. + + # Karma test related files: + "fixtures", + "karma_*.js", + "spec", + "spec_helpers", + "spec-helpers", + ] diff --git a/openedx/core/djangoapps/staticfiles/docs/decisions/0001-purpose-of-app.rst b/openedx/core/djangoapps/staticfiles/docs/decisions/0001-purpose-of-app.rst new file mode 100644 index 0000000000..d723363dc6 --- /dev/null +++ b/openedx/core/djangoapps/staticfiles/docs/decisions/0001-purpose-of-app.rst @@ -0,0 +1,43 @@ +Purpose of this app: staticfiles +################################ + +Status +****** + +**Accepted** + + +Context +******* + +Django provides the ``collectstatic`` management command, which collects static assets into the ``STATIC_ROOT`` so that they can be served by some system external to Django (like nginx or caddy), as is usually desired in production environments. + +edx-platform contains several types of files that we don't want to be collected into the ``STATIC_ROOT``. Previously, these files had to be supplied to the management command using the ``--ignore`` option:: + + ./manage.py lms collectstatic --ignore geoip --ignore sass ...etc + ./manage.py cms collectstatic --ignore geoip --ignore sass ...etc + +This yields a long, hard-to-remember command. Paver wrapped the command in its big ``paver update_assets`` task, but that task also builds assets, which is totally overkill when you're just trying to collect them. + +Fortunately, ``collectstatic``'s default ignore patterns can be configured by defining a custom AppConfig class. + +Decision +******** + +In this app, we define such an config (``EdxPlatformStaticFilesConfig``) for LMS and CMS. Now, devs can collect LMS & CMS assets with just:: + + ./manage.py lms collectstatic + ./manage.py cms collectstatic + +Going forward, this app could be used to further customize the Django staticfiles app, although at time of writing no further customizations are planned. + +Although the current guidance is to avoid creating new edx-platform apps, there is no other way to add this configuration to the platform. Because the ignore patterns are highly specific to edx-platform, creating this app as a separate library would not be useful. + +Further reading +*************** + +Django's ``collecstatic`` ignore behavior: https://docs.djangoproject.com/en/3.2/ref/contrib/staticfiles/#customizing-the-ignored-pattern-list + +Why we are moving away from Paver wrappers: https://github.com/openedx/edx-platform/blob/master/docs/decisions/0017-reimplement-asset-processing.rst + +Issue: https://github.com/openedx/edx-platform/issues/31658 diff --git a/pavelib/assets.py b/pavelib/assets.py index ad933de195..003c764875 100644 --- a/pavelib/assets.py +++ b/pavelib/assets.py @@ -692,30 +692,9 @@ def collect_assets(systems, settings, **kwargs): `settings` is the Django settings module to use. `**kwargs` include arguments for using a log directory for collectstatic output. Defaults to /dev/null. """ - ignore_patterns = [ - # Karma test related files... - "fixtures", - "karma_*.js", - "spec", - "spec_helpers", - "spec-helpers", - "xmodule_js", # symlink for tests - - # Geo-IP data, only accessed in Python - "geoip", - - # We compile these out, don't need the source files in staticfiles - "sass", - ] - - ignore_args = " ".join( - f'--ignore "{pattern}"' for pattern in ignore_patterns - ) - for sys in systems: collectstatic_stdout_str = _collect_assets_cmd(sys, **kwargs) - sh(django_cmd(sys, settings, "collectstatic {ignore_args} --noinput {logfile_str}".format( - ignore_args=ignore_args, + sh(django_cmd(sys, settings, "collectstatic --noinput {logfile_str}".format( logfile_str=collectstatic_stdout_str ))) print(f"\t\tFinished collecting {sys} assets.")