Skip to content

Commit

Permalink
Merge branch 'navin/component-collection-api' into rpenido/navin/comp…
Browse files Browse the repository at this point in the history
…onent-collection-api-fix-meilisearch
  • Loading branch information
navinkarkera committed Oct 12, 2024
2 parents 758334f + 8babec2 commit 46e577a
Show file tree
Hide file tree
Showing 16 changed files with 66 additions and 126 deletions.
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/[email protected]
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/pylint-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ on:

jobs:
run-pylint:
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
strategy:
fail-fast: false
matrix:
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/[email protected]
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

0 comments on commit 46e577a

Please sign in to comment.