From 5a25e7c8ca97788bcb4a86fac97e7dba94acd313 Mon Sep 17 00:00:00 2001 From: Alex Dusenbery Date: Mon, 10 Jun 2024 11:04:02 -0400 Subject: [PATCH 1/4] feat: edx-enterprise 4.19.14 | revert errant release, update mgmt command --- requirements/constraints.txt | 2 +- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 6806cf1240..2a7b6d6bb5 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -23,7 +23,7 @@ click>=8.0,<9.0 # The team that owns this package will manually bump this package rather than having it pulled in automatically. # This is to allow them to better control its deployment and to do it in a process that works better # for them. -edx-enterprise==4.19.11 +edx-enterprise==4.19.14 # Stay on LTS version, remove once this is added to common constraint Django<5.0 diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index d6518d0326..0a55b7e894 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -464,7 +464,7 @@ edx-drf-extensions==10.3.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.19.11 +edx-enterprise==4.19.14 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index d06e4a226f..5855fb2ebf 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -743,7 +743,7 @@ edx-drf-extensions==10.3.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.19.11 +edx-enterprise==4.19.14 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 73de164db0..2623f9c720 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -538,7 +538,7 @@ edx-drf-extensions==10.3.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.19.11 +edx-enterprise==4.19.14 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index c8b2ac7939..a1045c9766 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -571,7 +571,7 @@ edx-drf-extensions==10.3.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.19.11 +edx-enterprise==4.19.14 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt From 0eb61e28d1b10319546874807e38a1b685b9f71f Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Mon, 10 Jun 2024 11:44:46 -0400 Subject: [PATCH 2/4] feat: Start conversion of StaticContentServer from middleware into view (#34703) See https://github.com/openedx/edx-platform/issues/34702 This necessarily involves switching from calling `StaticContent.is_versioned_asset_path` to determine whether to handle the request to having a hardcoded urlpattern. I've made the choice to hardcode the other two patterns similarly rather than using imported constants. The mapping of URL patterns to database records should be explicit (even though we don't expect those constants to change out from under us.) I've renamed the middleware rather than choosing a new name for the implementation because there are other references in tests and other code. This was the smaller change. A note on HTTP methods: The middleware currently completely ignores the request's HTTP method, so I wanted to confirm that only GETs were being used in practice. This query reveals that 99.8% of requests that this middleware handles are GET, with just a smattering of PROPFIND and OPTIONS and a tiny number of HEAD and POST: ``` from Transaction select count(*) facet request.method where name = 'WebTransaction/Function/openedx.core.djangoapps.contentserver.middleware:StaticContentServer' since 4 weeks ago ``` --- cms/envs/common.py | 2 +- cms/urls.py | 3 + lms/envs/common.py | 2 +- lms/urls.py | 3 + .../djangoapps/contentserver/middleware.py | 32 +++++++++- openedx/core/djangoapps/contentserver/urls.py | 16 +++++ .../core/djangoapps/contentserver/views.py | 58 +++++++++++++++++++ 7 files changed, 113 insertions(+), 3 deletions(-) create mode 100644 openedx/core/djangoapps/contentserver/urls.py create mode 100644 openedx/core/djangoapps/contentserver/views.py diff --git a/cms/envs/common.py b/cms/envs/common.py index 24a63fb7d9..e2ad3e239f 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -948,7 +948,7 @@ MIDDLEWARE = [ 'openedx.core.djangoapps.cache_toolbox.middleware.CacheBackedAuthenticationMiddleware', 'common.djangoapps.student.middleware.UserStandingMiddleware', - 'openedx.core.djangoapps.contentserver.middleware.StaticContentServer', + 'openedx.core.djangoapps.contentserver.middleware.StaticContentServerMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'common.djangoapps.track.middleware.TrackMiddleware', diff --git a/cms/urls.py b/cms/urls.py index 9828e9d0fb..e21d07083e 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -74,6 +74,9 @@ urlpatterns = oauth2_urlpatterns + [ path('heartbeat', include('openedx.core.djangoapps.heartbeat.urls')), path('i18n/', include('django.conf.urls.i18n')), + # Course assets + path('', include('openedx.core.djangoapps.contentserver.urls')), + # User API endpoints path('api/user/', include('openedx.core.djangoapps.user_api.urls')), diff --git a/lms/envs/common.py b/lms/envs/common.py index 7fbddadf38..3074d958b2 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2287,7 +2287,7 @@ MIDDLEWARE = [ 'openedx.core.djangoapps.safe_sessions.middleware.EmailChangeMiddleware', 'common.djangoapps.student.middleware.UserStandingMiddleware', - 'openedx.core.djangoapps.contentserver.middleware.StaticContentServer', + 'openedx.core.djangoapps.contentserver.middleware.StaticContentServerMiddleware', # Adds user tags to tracking events # Must go before TrackMiddleware, to get the context set up diff --git a/lms/urls.py b/lms/urls.py index 5ac6283fdd..48b0aecaeb 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -111,6 +111,9 @@ urlpatterns = [ path('i18n/', include('django.conf.urls.i18n')), + # Course assets + path('', include('openedx.core.djangoapps.contentserver.urls')), + # Enrollment API RESTful endpoints path('api/enrollment/v1/', include('openedx.core.djangoapps.enrollments.urls')), diff --git a/openedx/core/djangoapps/contentserver/middleware.py b/openedx/core/djangoapps/contentserver/middleware.py index f159a00a56..6eb72da458 100644 --- a/openedx/core/djangoapps/contentserver/middleware.py +++ b/openedx/core/djangoapps/contentserver/middleware.py @@ -16,6 +16,7 @@ from django.http import ( ) from django.utils.deprecation import MiddlewareMixin from edx_django_utils.monitoring import set_custom_attribute +from edx_toggles.toggles import WaffleFlag from opaque_keys import InvalidKeyError from opaque_keys.edx.locator import AssetLocator @@ -32,6 +33,18 @@ from .models import CdnUserAgentsConfig, CourseAssetCacheTtlConfig log = logging.getLogger(__name__) +# .. toggle_name: content_server.use_view +# .. toggle_implementation: WaffleFlag +# .. toggle_default: False +# .. toggle_description: Deployment flag for switching asset serving from a middleware +# to a view. Intended to be used once in each environment to test the cutover and +# ensure there are no errors or changes in behavior. Once this has been tested, +# the middleware can be fully converted to a view. +# .. toggle_use_cases: temporary +# .. toggle_creation_date: 2024-05-02 +# .. toggle_target_removal_date: 2024-07-01 +# .. toggle_tickets: https://github.com/openedx/edx-platform/issues/34702 +CONTENT_SERVER_USE_VIEW = WaffleFlag('content_server.use_view', module_name=__name__) # TODO: Soon as we have a reasonable way to serialize/deserialize AssetKeys, we need # to change this file so instead of using course_id_partial, we're just using asset keys @@ -39,12 +52,26 @@ log = logging.getLogger(__name__) HTTP_DATE_FORMAT = "%a, %d %b %Y %H:%M:%S GMT" -class StaticContentServer(MiddlewareMixin): +class StaticContentServerMiddleware(MiddlewareMixin): + """ + Shim to maintain old pattern of serving course assets from a middleware. See views.py. + """ + def process_request(self, request): + """Intercept asset request or allow view to handle it, depending on config.""" + if CONTENT_SERVER_USE_VIEW.is_enabled(): + return + else: + set_custom_attribute('content_server.handled_by.middleware', True) + return IMPL.process_request(request) + + +class StaticContentServer(): """ Serves course assets to end users. Colloquially referred to as "contentserver." """ def is_asset_request(self, request): """Determines whether the given request is an asset request""" + # Don't change this without updating urls.py! See docstring of views.py. return ( request.path.startswith('/' + XASSET_LOCATION_TAG + '/') or @@ -295,6 +322,9 @@ class StaticContentServer(MiddlewareMixin): return content +IMPL = StaticContentServer() + + def parse_range_header(header_value, content_length): """ Returns the unit and a list of (start, end) tuples of ranges. diff --git a/openedx/core/djangoapps/contentserver/urls.py b/openedx/core/djangoapps/contentserver/urls.py new file mode 100644 index 0000000000..96bbe6bf38 --- /dev/null +++ b/openedx/core/djangoapps/contentserver/urls.py @@ -0,0 +1,16 @@ +""" +URL patterns for course asset serving. +""" + +from django.urls import path, re_path + +from . import views + +# These patterns are incomplete and do not capture the variable +# components of the URLs. That's because the view itself is separately +# parsing the paths, for historical reasons. See docstring on views.py. +urlpatterns = [ + path("c4x/", views.course_assets_view), + re_path("^asset-v1:", views.course_assets_view), + re_path("^assets/courseware/", views.course_assets_view), +] diff --git a/openedx/core/djangoapps/contentserver/views.py b/openedx/core/djangoapps/contentserver/views.py new file mode 100644 index 0000000000..84bd4c3b10 --- /dev/null +++ b/openedx/core/djangoapps/contentserver/views.py @@ -0,0 +1,58 @@ +""" +Views for serving course assets. + +Historically, this was implemented as a *middleware* (StaticContentServer) that +intercepted requests with paths matching certain patterns, rather than using +urlpatterns and a view. There wasn't any good reason for this, as far as I can +tell. It causes some problems for telemetry: When the code-owner middleware asks +Django what view handled the request, it does so by looking at the result of the +`resolve` utility, but these URLs get a Resolver404 (because there's no +registered urlpattern). + +We'd like to turn this into a proper view: +https://github.com/openedx/edx-platform/issues/34702 + +The first step, seen here, is to have urlpatterns (redundant with the +middleware's `is_asset_request` method) and a view, but the view just calls into +the same code the middleware uses. The implementation of the middleware has been +moved into StaticContentServerImpl, leaving the middleware as just a shell +around the latter. + +A waffle flag chooses whether to allow the middleware to handle the request, or +whether to pass the request along to the view. Why? Because we might be relying +by accident on some weird behavior inherent to misusing a middleware this way, +and we need a way to quickly switch back if we encounter problems. + +If the view works, we can move all of StaticContentServerImpl directly into the +view and drop the middleware and the waffle flag. +""" +from django.http import HttpResponseNotFound +from django.views.decorators.http import require_safe +from edx_django_utils.monitoring import set_custom_attribute + +from .middleware import CONTENT_SERVER_USE_VIEW, IMPL + + +@require_safe +def course_assets_view(request): + """ + Serve course assets to end users. Colloquially referred to as "contentserver." + """ + set_custom_attribute('content_server.handled_by.view', True) + + if not CONTENT_SERVER_USE_VIEW.is_enabled(): + # Should never happen; keep track of occurrences. + set_custom_attribute('content_server.view.called_when_disabled', True) + # But handle the request anyhow. + + # We'll delegate request handling to an instance of the middleware + # until we can verify that the behavior is identical when requests + # come all the way through to the view. + response = IMPL.process_request(request) + + if response is None: + # Shouldn't happen + set_custom_attribute('content_server.view.no_response_from_impl', True) + return HttpResponseNotFound() + else: + return response From a3bafb2ccd6b0920bf2fd4e05b2b7e5add8e682e Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Mon, 10 Jun 2024 15:50:06 -0400 Subject: [PATCH 3/4] temp: build: compile python requirements with 3.8, not 3.11 (#34960) We haven't quite dropped Py3.8 support yet, so we can't use these workflows if they're compiling with 3.11: * upgrade-one-python-dependency (upgrading a single, targeted dep) * compile-python-requirements (recompiling all deps without upgrading) So, we "downgrade" those workflows to use 3.8, for now. We should revert this commit when we drop 3.8 support. Note: The upgrade-python-requirements.yml workflow is still using 3.8, so this commit will update the other workflows to match that one. --- .github/workflows/compile-python-requirements.yml | 2 +- .github/workflows/upgrade-one-python-dependency.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/compile-python-requirements.yml b/.github/workflows/compile-python-requirements.yml index 0ff99b9c68..fda1d3b338 100644 --- a/.github/workflows/compile-python-requirements.yml +++ b/.github/workflows/compile-python-requirements.yml @@ -26,7 +26,7 @@ jobs: - name: Set up Python environment uses: actions/setup-python@v5 with: - python-version: "3.11" + python-version: "3.8" - name: Run make compile-requirements env: diff --git a/.github/workflows/upgrade-one-python-dependency.yml b/.github/workflows/upgrade-one-python-dependency.yml index 6ca5dfcb35..baed246246 100644 --- a/.github/workflows/upgrade-one-python-dependency.yml +++ b/.github/workflows/upgrade-one-python-dependency.yml @@ -39,7 +39,7 @@ jobs: - name: Set up Python environment uses: actions/setup-python@v5 with: - python-version: "3.11" + python-version: "3.8" - name: Update any pinned dependencies env: From b98cbd4c2cddec4e5daf9defa74d96c2d4accc99 Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Wed, 13 Mar 2024 21:33:03 -0300 Subject: [PATCH 4/4] feat: warn when relative dates are past due and can't be shifted --- lms/templates/problem.html | 16 +++++++++------- lms/templates/vert_module.html | 2 +- openedx/features/course_experience/utils.py | 10 ++++++++-- .../call_to_action.py | 16 ++++++++++++---- 4 files changed, 30 insertions(+), 14 deletions(-) diff --git a/lms/templates/problem.html b/lms/templates/problem.html index b785e4aa68..e239c32cd2 100644 --- a/lms/templates/problem.html +++ b/lms/templates/problem.html @@ -68,7 +68,8 @@ from openedx.core.djangolib.markup import HTML, Text (${submit_disabled_cta['description']}) % else: -
+ + % if submit_disabled_cta.get('link'): % for form_name, form_value in submit_disabled_cta['form_values'].items(): @@ -76,13 +77,14 @@ from openedx.core.djangolib.markup import HTML, Text - - + % endif + + - (${submit_disabled_cta['description']}) - + + (${submit_disabled_cta['description']}) + % endif % endif
diff --git a/lms/templates/vert_module.html b/lms/templates/vert_module.html index 0e52e3c7f4..7df5b8b5b4 100644 --- a/lms/templates/vert_module.html +++ b/lms/templates/vert_module.html @@ -44,7 +44,7 @@ from openedx.core.djangolib.markup import HTML - % else: + % elif vertical_banner_cta.get('link'):