diff --git a/.github/workflows/ci-static-analysis.yml b/.github/workflows/ci-static-analysis.yml index a3b0527aad72..458e00fc6b1f 100644 --- a/.github/workflows/ci-static-analysis.yml +++ b/.github/workflows/ci-static-analysis.yml @@ -10,7 +10,7 @@ jobs: matrix: python-version: - "3.11" - os: ["ubuntu-latest"] + os: ["ubuntu-22.04"] steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/migrations-check.yml b/.github/workflows/migrations-check.yml index f253d48e4f41..624caddd5309 100644 --- a/.github/workflows/migrations-check.yml +++ b/.github/workflows/migrations-check.yml @@ -13,7 +13,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-latest] + os: [ubuntu-22.04] python-version: - "3.11" # 'pinned' is used to install the latest patch version of Django @@ -126,7 +126,7 @@ jobs: if: always() needs: - check_migrations - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 steps: - name: Decide whether the needed jobs succeeded or failed # uses: re-actors/alls-green@v1.2.1 diff --git a/.github/workflows/pylint-checks.yml b/.github/workflows/pylint-checks.yml index 144cc77a3da4..ad3aad388780 100644 --- a/.github/workflows/pylint-checks.yml +++ b/.github/workflows/pylint-checks.yml @@ -8,7 +8,7 @@ on: jobs: run-pylint: - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 strategy: fail-fast: false matrix: @@ -75,7 +75,7 @@ jobs: if: always() needs: - run-pylint - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 steps: - name: Decide whether the needed jobs succeeded or failed # uses: re-actors/alls-green@v1.2.1 diff --git a/.github/workflows/quality-checks.yml b/.github/workflows/quality-checks.yml index 5445d70e3b4b..84610123493c 100644 --- a/.github/workflows/quality-checks.yml +++ b/.github/workflows/quality-checks.yml @@ -13,7 +13,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-latest] + os: [ubuntu-22.04] python-version: - "3.11" node-version: [20] diff --git a/.github/workflows/static-assets-check.yml b/.github/workflows/static-assets-check.yml index 0a417f9b1c79..4fe66e2a7778 100644 --- a/.github/workflows/static-assets-check.yml +++ b/.github/workflows/static-assets-check.yml @@ -12,7 +12,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-latest] + os: [ubuntu-22.04] python-version: - "3.11" node-version: [18, 20] diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 5fef1c8352ce..854677b93cff 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -15,7 +15,7 @@ concurrency: jobs: run-tests: name: ${{ matrix.shard_name }}(py=${{ matrix.python-version }},dj=${{ matrix.django-version }},mongo=${{ matrix.mongo-version }}) - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 strategy: matrix: python-version: @@ -164,7 +164,7 @@ jobs: overwrite: true collect-and-verify: - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v4 - name: Setup Python @@ -229,7 +229,7 @@ jobs: # https://github.com/orgs/community/discussions/33579 success: name: Unit tests successful - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 if: always() needs: [run-tests] steps: @@ -240,7 +240,7 @@ jobs: jobs: ${{ toJSON(needs) }} compile-warnings-report: - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 needs: [run-tests] steps: - uses: actions/checkout@v4 @@ -268,7 +268,7 @@ jobs: overwrite: true merge-artifacts: - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 needs: [compile-warnings-report] steps: - name: Merge Pytest Warnings JSON Artifacts @@ -288,7 +288,7 @@ jobs: # Combine and upload coverage reports. coverage: if: (github.repository == 'edx/edx-platform-private') || (github.repository == 'openedx/edx-platform' && (startsWith(github.base_ref, 'open-release') == false)) - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 needs: [run-tests] strategy: matrix: diff --git a/openedx/core/djangoapps/xblock/api.py b/openedx/core/djangoapps/xblock/api.py index 43ec3909bcd6..dbb7c824676b 100644 --- a/openedx/core/djangoapps/xblock/api.py +++ b/openedx/core/djangoapps/xblock/api.py @@ -25,13 +25,13 @@ from xblock.exceptions import NoSuchUsage, NoSuchViewError from xblock.plugin import PluginMissingError +from openedx.core.types import User as UserType from openedx.core.djangoapps.xblock.apps import get_xblock_app_config from openedx.core.djangoapps.xblock.learning_context.manager import get_learning_context_impl from openedx.core.djangoapps.xblock.runtime.learning_core_runtime import ( LearningCoreFieldData, LearningCoreXBlockRuntime, ) -from openedx.core.djangoapps.xblock.runtime.runtime import XBlockRuntimeSystem as _XBlockRuntimeSystem from .utils import get_secure_token_for_xblock_handler, get_xblock_id_for_anonymous_user from .runtime.learning_core_runtime import LearningCoreXBlockRuntime @@ -54,33 +54,21 @@ class CheckPerm(Enum): CAN_EDIT = 3 -def get_runtime_system(): +def get_runtime(user: UserType): """ - Return a new XBlockRuntimeSystem. - - TODO: Refactor to get rid of the XBlockRuntimeSystem entirely and just - create the LearningCoreXBlockRuntime and return it. We used to want to keep - around a long lived runtime system (a factory that returns runtimes) for - caching purposes, and have it dynamically construct a runtime on request. - Now we're just re-constructing both the system and the runtime in this call - and returning it every time, because: - - 1. We no longer have slow, Blockstore-style definitions to cache, so the - performance of this is perfectly acceptable. - 2. Having a singleton increases complexity and the chance of bugs. - 3. Creating the XBlockRuntimeSystem every time only takes about 10-30 µs. - - Given that, the extra XBlockRuntimeSystem class just adds confusion. But - despite that, it's tested, working code, and so I'm putting off refactoring - for now. + Return a new XBlockRuntime. + + Each XBlockRuntime is bound to one user (and usually one request or one + celery task). It is typically used just to load and render a single block, + but the API _does_ allow a single runtime instance to load multiple blocks + (as long as they're for the same user). """ - params = get_xblock_app_config().get_runtime_system_params() + params = get_xblock_app_config().get_runtime_params() params.update( - runtime_class=LearningCoreXBlockRuntime, handler_url=get_handler_url, authored_data_store=LearningCoreFieldData(), ) - runtime = _XBlockRuntimeSystem(**params) + runtime = LearningCoreXBlockRuntime(user, **params) return runtime @@ -121,7 +109,7 @@ def load_block(usage_key, user, *, check_permission: CheckPerm | None = CheckPer # e.g. a course might specify that all 'problem' XBlocks have 'max_attempts' # set to 3. # field_overrides = context_impl.get_field_overrides(usage_key) - runtime = get_runtime_system().get_runtime(user=user) + runtime = get_runtime(user=user) try: return runtime.get_block(usage_key) diff --git a/openedx/core/djangoapps/xblock/apps.py b/openedx/core/djangoapps/xblock/apps.py index 5ba2361322ec..848470a3a90c 100644 --- a/openedx/core/djangoapps/xblock/apps.py +++ b/openedx/core/djangoapps/xblock/apps.py @@ -16,9 +16,9 @@ class XBlockAppConfig(AppConfig): verbose_name = 'New XBlock Runtime' label = 'xblock_new' # The name 'xblock' is already taken by ORA2's 'openassessment.xblock' app :/ - def get_runtime_system_params(self): + def get_runtime_params(self): """ - Get the XBlockRuntimeSystem parameters appropriate for viewing and/or + Get the LearningCoreXBlockRuntime parameters appropriate for viewing and/or editing XBlock content. """ raise NotImplementedError @@ -43,9 +43,9 @@ class LmsXBlockAppConfig(XBlockAppConfig): LMS-specific configuration of the XBlock Runtime django app. """ - def get_runtime_system_params(self): + def get_runtime_params(self): """ - Get the XBlockRuntimeSystem parameters appropriate for viewing and/or + Get the LearningCoreXBlockRuntime parameters appropriate for viewing and/or editing XBlock content in the LMS """ return dict( @@ -65,9 +65,9 @@ class StudioXBlockAppConfig(XBlockAppConfig): Studio-specific configuration of the XBlock Runtime django app. """ - def get_runtime_system_params(self): + def get_runtime_params(self): """ - Get the XBlockRuntimeSystem parameters appropriate for viewing and/or + Get the LearningCoreXBlockRuntime parameters appropriate for viewing and/or editing XBlock content in Studio """ return dict( diff --git a/openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py b/openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py index 26aa7af60f0b..41fd79f5a068 100644 --- a/openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py +++ b/openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py @@ -211,7 +211,7 @@ def get_block(self, usage_key, for_parent=None): # We've pre-loaded the fields for this block, so the FieldData shouldn't # consider these values "changed" in its sense of "you have to persist # these because we've altered the field values from what was stored". - self.system.authored_data_store.mark_unchanged(block) + self.authored_data_store.mark_unchanged(block) return block @@ -221,7 +221,7 @@ def save_block(self, block): This gets called by block.save() - do not call this directly. """ - if not self.system.authored_data_store.has_changes(block): + if not self.authored_data_store.has_changes(block): return # No changes, so no action needed. # Verify that the user has permission to write to authored data in this @@ -254,7 +254,7 @@ def save_block(self, block): }, created=now, ) - self.system.authored_data_store.mark_unchanged(block) + self.authored_data_store.mark_unchanged(block) def _get_component_from_usage_key(self, usage_key): """ diff --git a/openedx/core/djangoapps/xblock/runtime/runtime.py b/openedx/core/djangoapps/xblock/runtime/runtime.py index 5746af491d10..fe633f686f02 100644 --- a/openedx/core/djangoapps/xblock/runtime/runtime.py +++ b/openedx/core/djangoapps/xblock/runtime/runtime.py @@ -95,18 +95,30 @@ class XBlockRuntime(RuntimeShim, Runtime): # currently only used to track if we're in the studio_view (see below under service()) view_name: str | None - def __init__(self, system: XBlockRuntimeSystem, user: UserType | None): + def __init__( + self, + user: UserType | None, + *, + handler_url: Callable[[UsageKey, str, UserType | None], str], + student_data_mode: StudentDataMode, + id_reader: Optional[IdReader] = None, + authored_data_store: Optional[FieldData] = None, + ): super().__init__( - id_reader=system.id_reader, + id_reader=id_reader or OpaqueKeyReader(), mixins=( LmsBlockMixin, # Adds Non-deprecated LMS/Studio functionality XBlockShim, # Adds deprecated LMS/Studio functionality / backwards compatibility ), default_class=None, select=None, - id_generator=system.id_generator, + id_generator=MemoryIdManager(), # We don't really use id_generator until we need to support asides ) - self.system = system + assert student_data_mode in (StudentDataMode.Ephemeral, StudentDataMode.Persisted) + self.authored_data_store = authored_data_store + self.children_data_store = None + self.student_data_mode = student_data_mode + self.handler_url_fn = handler_url self.user = user # self.user_id must be set as a separate attribute since base class sets it: if self.user is None: @@ -126,7 +138,7 @@ def handler_url(self, block, handler_name: str, suffix='', query='', thirdparty= if thirdparty: log.warning("thirdparty handlers are not supported by this runtime for XBlock %s.", type(block)) - url = self.system.handler_url(block.scope_ids.usage_id, handler_name, self.user) + url = self.handler_url_fn(block.scope_ids.usage_id, handler_name, self.user) if suffix: if not url.endswith('/'): url += '/' @@ -275,7 +287,7 @@ def service(self, block: XBlock, service_name: str): # the preview engine, and 'main' otherwise. # For backwards compatibility, we check the student_data_mode (Ephemeral indicates CMS) and the # view_name for 'studio_view.' self.view_name is set by render() below. - if self.system.student_data_mode == StudentDataMode.Ephemeral and self.view_name != 'studio_view': + if self.student_data_mode == StudentDataMode.Ephemeral and self.view_name != 'studio_view': return MakoService(namespace_prefix='lms.') return MakoService() elif service_name == "i18n": @@ -301,14 +313,12 @@ def service(self, block: XBlock, service_name: str): return EventPublishingService(self.user, context_key, make_track_function()) elif service_name == 'enrollments': return EnrollmentsService() + elif service_name == 'error_tracker': + return make_error_tracker() - # Check if the XBlockRuntimeSystem wants to handle this: - service = self.system.get_service(block, service_name) # Otherwise, fall back to the base implementation which loads services # defined in the constructor: - if service is None: - service = super().service(block, service_name) - return service + return super().service(block, service_name) def _init_field_data_for_block(self, block: XBlock) -> FieldData: """ @@ -322,7 +332,7 @@ def _init_field_data_for_block(self, block: XBlock) -> FieldData: assert isinstance(self.user_id, str) and self.user_id.startswith("anon") kvs = EphemeralKeyValueStore() student_data_store = KvsFieldData(kvs) - elif self.system.student_data_mode == StudentDataMode.Ephemeral: + elif self.student_data_mode == StudentDataMode.Ephemeral: # We're in an environment like Studio where we want to let the # author test blocks out but not permanently save their state. kvs = EphemeralKeyValueStore() @@ -341,10 +351,10 @@ def _init_field_data_for_block(self, block: XBlock) -> FieldData: student_data_store = KvsFieldData(kvs=DjangoKeyValueStore(field_data_cache)) return SplitFieldData({ - Scope.content: self.system.authored_data_store, - Scope.settings: self.system.authored_data_store, - Scope.parent: self.system.authored_data_store, - Scope.children: self.system.children_data_store, + Scope.content: self.authored_data_store, + Scope.settings: self.authored_data_store, + Scope.parent: self.authored_data_store, + Scope.children: self.children_data_store, Scope.user_state_summary: student_data_store, Scope.user_state: student_data_store, Scope.user_info: student_data_store, @@ -407,62 +417,3 @@ def _lookup_asset_url(self, block: XBlock, asset_path: str): # pylint: disable= """ # Subclasses should override this return None - - -class XBlockRuntimeSystem: - """ - This class is essentially a factory for XBlockRuntimes. This is a - long-lived object which provides the behavior specific to the application - that wants to use XBlocks. Unlike XBlockRuntime, a single instance of this - class can be used with many different XBlocks, whereas each XBlock gets its - own instance of XBlockRuntime. - """ - def __init__( - self, - handler_url: Callable[[UsageKey, str, UserType | None], str], - student_data_mode: StudentDataMode, - runtime_class: type[XBlockRuntime], - id_reader: Optional[IdReader] = None, - authored_data_store: Optional[FieldData] = None, - ): - """ - args: - handler_url: A method to get URLs to call XBlock handlers. It must - implement this signature: - handler_url( - usage_key: UsageKey, - handler_name: str, - user: User | AnonymousUser | None - ) -> str - student_data_mode: Specifies whether student data should be kept - in a temporary in-memory store (e.g. Studio) or persisted - forever in the database. - runtime_class: What runtime to use, e.g. LearningCoreXBlockRuntime - """ - self.handler_url = handler_url - self.id_reader = id_reader or OpaqueKeyReader() - self.id_generator = MemoryIdManager() # We don't really use id_generator until we need to support asides - self.runtime_class = runtime_class - self.authored_data_store = authored_data_store - self.children_data_store = None - assert student_data_mode in (StudentDataMode.Ephemeral, StudentDataMode.Persisted) - self.student_data_mode = student_data_mode - - def get_runtime(self, user: UserType | None) -> XBlockRuntime: - """ - Get the XBlock runtime for the specified Django user. The user can be - a regular user, an AnonymousUser, or None. - """ - return self.runtime_class(self, user) - - def get_service(self, block, service_name: str): - """ - Get a runtime service - - Runtime services may come from this XBlockRuntimeSystem, - or if this method returns None, they may come from the - XBlockRuntime. - """ - if service_name == 'error_tracker': - return make_error_tracker() - return None # None means see if XBlockRuntime offers this service diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 6535ebc5ab2a..f4d8581c5393 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -82,7 +82,7 @@ django-storages<1.14.4 # 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.27.2 +edx-enterprise==4.27.3 # Date: 2024-05-09 # This has to be constrained as well because newer versions of edx-i18n-tools need the diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 5eea20268e5c..5d9361542136 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -467,7 +467,7 @@ edx-drf-extensions==10.4.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.27.2 +edx-enterprise==4.27.3 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index a40c6d87dbd4..2414ac3ffdca 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -741,7 +741,7 @@ edx-drf-extensions==10.4.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.27.2 +edx-enterprise==4.27.3 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index e4db7f5e4314..2f6cec0cd6e2 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -547,7 +547,7 @@ edx-drf-extensions==10.4.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.27.2 +edx-enterprise==4.27.3 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/kernel.in b/requirements/edx/kernel.in index a5b510742ac7..d399f938cfdb 100644 --- a/requirements/edx/kernel.in +++ b/requirements/edx/kernel.in @@ -117,7 +117,8 @@ olxcleaner openedx-atlas # CLI tool to manage translations openedx-calc # Library supporting mathematical calculations for Open edX openedx-django-require -openedx-events # Open edX Events from Hooks Extension Framework (OEP-50) +# openedx-events # Open edX Events from Hooks Extension Framework (OEP-50) +openedx-events @ git+https://github.com/open-craft/openedx-events@navin/update-library-collection-data openedx-filters # Open edX Filters from Hooks Extension Framework (OEP-50) openedx-learning # Open edX Learning core (experimental) openedx-mongodbproxy diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 4f88fade88b5..1e1ac4874d09 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -571,7 +571,7 @@ edx-drf-extensions==10.4.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.27.2 +edx-enterprise==4.27.3 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt