Skip to content
Open
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
2 changes: 1 addition & 1 deletion .github/workflows/ci-static-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ jobs:
matrix:
python-version:
- "3.11"
os: ["ubuntu-latest"]
os: ["ubuntu-22.04"]

steps:
- uses: actions/checkout@v4
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/migrations-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/pylint-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ on:

jobs:
run-pylint:
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
strategy:
fail-fast: false
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/"
- module-name: openedx-2
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/quality-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/static-assets-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
12 changes: 6 additions & 6 deletions .github/workflows/unit-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down
34 changes: 11 additions & 23 deletions openedx/core/djangoapps/xblock/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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)
Expand Down
12 changes: 6 additions & 6 deletions openedx/core/djangoapps/xblock/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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):
"""
Expand Down
Loading