After we merged this PR: https://github.com/openedx/edx-platform/pull/33920
this error began popping up in logs:
Unable to load XBlock 'staffgradedxblock'
....
ImportError: cannot import name 'get_course_blocks' from
partially initialized module 'lms.djangoapps.course_blocks.api'
(most likely due to a circular import) ...
The root cause was the new imports of `derived_key` and `BlockKey` into
xmodule/library_content_block.py. Those new imports come from
xmodule/modulestore/store_utilities.py, which runs
`XBlock.load_classes()` at the module level, which fails because we are
still in the process of loading xmodule/library_content_block.
As a solution, we move both `derived_key` and `BlockKey` to
xmodule/util/keys.py. We could potentially move that file to opaque-keys
eventually, depending on how well we think that those concepts generalize.
Also:
* We rename the function from derived_key to derive_key, as
functions should be verbs.
* We combine the first to parameters of derive_key (a source ContextKey
and a source BlockKey) into a single parameter (a source UsageKey). In
my opinion, this makes the function call easier to understand.
BREAKING CHANGE: Removes all code, tests, and settings related to
indexing of V2 (blockstore-backed) content libraries in elasticsearch.
This includes indexing of top-level library metadata as well as indexing
of library block metadata. Operators who enabled the experimental
Library Authoring MFE *and* the experimental ENABLE_CONTENT_LIBRARY_INDEX
feature may notice that sorting, filtering, and searching of V2
libraries and their blocks may now be slower and/or less powerful.
The ENABLE_CONTENT_LIBRARY_INDEX feature was already disabled by
default, so most/all operators (including edx.org) should not notice
any difference.
Removed settings include:
* FEATURES['ENABLE_CONTENT_LIBRARY_INDEX']
* ENABLE_ELASTICSEARCH_FOR_TESTS
* TEST_ELASTICSEARCH_USE_SSL
* TEST_ELASTICSEARCH_HOST
* TEST_ELASTICSEARCH_PORT
For rationale, see the updated "Status" section of:
./openedx/core/djangoapps/content_libraries/docs/decisions/0001-index-libraries-in-elasticsearch.rst
Originally, Blockstore was an independent micro-service, accessed via a REST API.
Then, we changed Blockstore so it could be installed as an in-process Django app.
To support both modes, there existed a blockstore_api wrapper library in edx-platform,
with toggles controlling whether the wrapper called out to the micro-service's REST API versus the
Django app's Python API. Now that the micro-service Blockstore implementation is deprecated,
though, this wrapper library and toggles are just unnecessary complexity.
As a first step towards cleanup, we:
* remove several toggles and settings (details below);
* remove the blocokstore_api wrapper methods which called the REST API and
marshalled them back into Python objects; and
* remove all test cases which relied on the Blockstore micro-service (and were skippped in CI).
In the future, we will remove the content libraries indexer,
clean up the remaining bits of blockstore_api, and flatten out all
the Blockstore-related test class hierarchies which are no longer nceessary.
BREAKING CHANGE:
* These Django settings are removed:
* BLOCKSTORE_PUBLIC_URL_ROOT
* BLOCKSTORE_API_URL
* BLOCKSTORE_API_AUTH_TOKEN
* BLOCKSTORE_USE_BLOCKSTORE_APP_API
* The blockstore.use_blockstore_app_api Waffle switch is removed.
* edx-platform will act as it did when the DJango setting BLOCKSTORE_USE_BLOCKSTORE_APP_API
or the Waffle switch blockstore.use_blockstore_app_api were enabled. That is, any running Blockstore
micro-service instance will be ignored, and the Blockstore package which is installed into edx-platform
will be used instead.
Ref: https://github.com/openedx/blockstore/issues/296
After refactoring the library_content block to use asynchronous tasks
for syncing and duplicating children, we are seeing an error arise
during library_content duplication process on edx.org:
Traceback (most recent call last):
File "/edx/app/edxapp/edx-platform/openedx/core/djangoapps/content_libraries/tasks.py", line 305, in duplicate_children
_copy_overrides(store=store, user_id=user_id, source_block=source_block, dest_block=dest_block)
File "/edx/app/edxapp/edx-platform/openedx/core/djangoapps/content_libraries/tasks.py", line 383, in _copy_overrides
dest_block=dest_block.runtime.get_block(dest_child_key),
File "/edx/app/edxapp/edx-platform/xmodule/x_module.py", line 1401, in get_block
block = self.load_item(usage_id, for_parent=for_parent)
File "/edx/app/edxapp/edx-platform/xmodule/modulestore/split_mongo/caching_descriptor_system.py", line 127, in _load_item
block_data = self.get_module_data(block_key, course_key)
File "/edx/app/edxapp/edx-platform/xmodule/modulestore/split_mongo/caching_descriptor_system.py", line 154, in get_module_data
raise ItemNotFoundError(block_key)
xmodule.modulestore.exceptions.ItemNotFoundError: BlockKey(...)
We cannot reproduce the issue locally.
We are not entirely certain the cause of this, but we think it might
have do with caching. Specifically, the `store.get_item` and
`source_block.runtime.get_block` methods might use a different cache
than `dest_block.runtime.get_block`. It's possible that writes to
Mongo are sitting in one of those caches, causing reads from the
`dest_block.runtime`'s cache to fail to find dest_block's children.
We attempt to fix this by using the same "block getting" method
consistently. So instead of using a mix of `store.get_item`,
`source_block.runtime.get_block`, and `dest_block.runtime.get_block`,
we just use `store.get_item` everywhere.
Refactors and reworks the LibraryContentBlock so that its
sync-from-library operations are asynchronous and work with
V2 content libraries. This also required us to make
library_content block duplication asynchronous, as that
involves syncing from the source library.
For the sake of clarity, this PR includes two major method renames:
* update_children(...) -> sync_from_library(...)
* refresh_library(...) -> sync_from_library(upgrade_to_latest=True, ...)
an an XBlock HTTP handler rename:
/refresh_children -> /upgrade_and_sync
There are still a couple issues with import or duplication
of library_content blocks referencing V2 libraries other than
latest. These will be resolved in an upcoming PR.
Part of: https://openedx.atlassian.net/wiki/spaces/COMM/pages/3820617729/Spec+Memo+Content+Library+Authoring+Experience+V2
Follow-up work: https://github.com/openedx/edx-platform/issues/33640
Co-authored-by: Connor Haugh <chaugh@2u.com>
Co-authored-by: Eugene Dyudyunov <evgen.dyudyunov@raccoongang.com>
* chore: typing + fixes for content_staging
* chore: typing + fixes for learning_sequences
* chore: typing + fixes for content_libraries
* chore: typing + fixes for new XBlock runtime
* feat: type hinting more code with mypy
* feat: add xblock endpoint for updating an xblock
fix: remove debugger
feat: make function call more generic
refactor: just use request.json for request data as before
refactor: extract method
fix: revert wrong method change
fix: refactor correct method
feat: use handle_xblock method so that we can do more than update xblocks
fix: usage_key_string defaults to None
add all CRUD operations
fix usage key parameter
refactor: create /views folder
refactor: move xblock view functions to xblock_services
fix: tests
fix: tests
refactor: move xblock API endpoint to contentstore
* docs: add explanatory comment to new xblock_service
* feat: add feature flag for enabling content editing api
* feat: raise 404 if studio content api is disabled
* tests: test xblock endpoint
* test: make all post tests work
* test: check that xblock_handler receives correct args
* refactor: create util mixin for course factories with staff
* refactor: extract course staff authorization tests
* refactor: extract tests to api view testcase class
* test: add get tests
* test: fix tests
* test: fix tests
* test: fix tests
* test: add all crud tests
* fix: refactor to fix tests
* fix: merge conflict
* fix: merge conflict
* fix: tests after merge
* fix: json request decorator
* fix: lint
* fix: lint
* fix: lint
* fix: lint
* fix: new test files
* fix: lint
* fix: lint and apply PR suggestions
* fix: lint
* fix: lint
* fix: lint
* fix: lint
* fix: lint
* fix: lint
* test: warn about dependencies from cms->openedx->lms and vice versa
* test: warn about importing from package's internal implementation code
* chore: Update some imports to use public APIs only
* chore: Update 'bookmarks' app to have stricter public API
* fix: we are sharing 'adapters' from olx_rest_api to content_staging
This will ensure that errors raised by these tasks will alert the right
team. `send_course_enrollment_email` is the one I set out to fix, but I
discovered a few others.
I located tasks that were missing decorators by running the following
search and visually inspecting the results, although semgrep might be able
to do better:
```
ack '^@.*task|^@set_code_owner_attribute' cms lms common openedx xmodule --ignore-dir=tests --python
```
Also, add more detailed explanation of why a couple of tasks can't use the
decorator. This should only be an issue on tasks inheriting from
UserTaskMixin, which in practice is just CourseExportTask and
CourseImportTask (and the apparently unused EnrollmentReadTask and
EnrollmentWriteTask), via UserTask.
Previously, CMS Django admin would demand that at least one
LTIConfig be associated with the new library. This is not
necessary, but Django thought it was, because the `blank`
flag was not enabled on the ContentLibrary model's
`authored_lti_config` many-to-many field.
This should not affect the experimental blockstore-based
library authoring frontend, which already allows libraries
to be created without any authorized_lti_configs.
This will not affect the existing modulestore-based
library authoring frontend, since it doesn't even use
any of the models in question.
A migration is included with this commit, but it should NOT
change MySQL schema, since `blank` is a Django-level validation
flag.
* fix: call blockstore APIs in atomic transactions.
To ensure database integry when using the blockstore APIs, the Content
Library views which invoke blockstore API methods are wrapped in
database transactions.
* fix: assert create_library is called inside an atomic transaction block
Rendering asset URLs requires HTML blocks to be associated with a course key.
This change allows HTML blocks to be associated with libraries and still
render without error.
Tests which @requires_blockstore (i.e. the Blockstore service) have
been made to run as a unit test using the installed Blockstore app, and
will be run by the platform CI.
The Blockstore service tests can still be run manually by setting
EDXAPP_RUN_BLOCKSTORE_TESTS=1
Related fixes:
* adds blockstore bundle storage settings
* let the studio devstack and test servers serve static files from
the /media URL This allows the blockstore/content libraries API to
serve blockstore assets in dev.
* Wrap ContentLibrary creation in an atomic transaction, so that if it
fails, the related bundle can be deleted directly from the database
during the exception handler. (Previously, we called a REST API which
deleted it as part of a separate service.)
* adds blockstore as a requirement and an installed app, with
configurable bundle storage settings.
* adds waffle switch and setting to allow use of blockstore's python API
instead of REST API in live testservers and in production.
* adds database router which, when a `blockstore` DATABASE connection is
configured, allows the platform to use the blockstore service's
database instead of the default edxapp database.
* replaces blockstore_api exceptions and models with blockstore.app.api classes
* minor fixes to the blockstore_api to make the Blockstore REST API
return data packaged the same as the Blockstore Python API.
Change `mark_user_change_as_expected` to no longer take the response object
and instead convey the expected-change information via RequestCache.
This requires edx-django-utils 4.4.2, which fixes the bug where
RequestCache was cleared in the exception phase.
Also, no longer mark `ENFORCE_SAFE_SESSIONS` toggle as
temporary. We'll want it as an opt-out.
I was tempted to take this opportunity to move any existing
`mark_user_change_as_expected` calls to be closer to where the actual
change request.user occurs, reducing risk of both false positives and false
negatives, but it would be better to do that one at a time in case a move
breaks something. (Ideally it would be called right after any
`django.contrib.auth` `login` or `logout` call; previously, we were
constrained by having to make the call after a response object had been
created.) These changes can be made later if it becomes necessary.
when fetching block metadata and rendering blocks while maintaining the
original usage IDs/OLX.
This change is marked temporary because LabXchange need it during the
transition to a custom runtime, but it's not really useful to anyone
else. We will revert this change with a future PR.
* chore: update deprecated import from collections
* chore: remove outdated imports from markdown library
as it hasn't been supported since 2.0.3 and we're on 3.x.
This was deprecated at least as early as 2012!
* docs: add docstring and remove lint-amnesty to markdown plugin
* chore: remove deprecated etree import
* style: remove unnecessary-comprehension for sets
* style: resolve a number of amnestied pylint complaints
Co-authored-by: stvn <stvn@mit.edu>
The lti user gets authenticated and logged in as a part of the LTI View
Post. This results in the request user changing during the request. We
mark it here so that it does not produce a false alarm in our
SafeSessions middleware.
If the bundle is deleted, `blockstore_cache.get_bundle_version_number(bundle_uuid)`
will raise an error.
Catch this error in `get_bundle_links`,
so the REST api doesn't crash with a 500 error when a linked bundle is deleted.
The functionality is exposed by (a.) A RESTful API, and (b.) A
management command. Both are added to Content Libraries V2. They allow
block importing from a local modulestore instance. Additionally, the
command line supports importing blocks from a remote platform instance
through API calls.
Additionally, fixes were added to parts of the system where needed to
properly export blocks to Content Libraries.
