diff --git a/openedx/core/djangoapps/xblock/runtime/blockstore_field_data.py b/openedx/core/djangoapps/xblock/runtime/blockstore_field_data.py index 517e93b3e2..d849bfcfb9 100644 --- a/openedx/core/djangoapps/xblock/runtime/blockstore_field_data.py +++ b/openedx/core/djangoapps/xblock/runtime/blockstore_field_data.py @@ -180,9 +180,7 @@ class BlockstoreFieldData(FieldData): # Otherwise, this is an anomalous get() before the XML was fully loaded: # This could happen if an XBlock's parse_xml() method tried to read a field before setting it, # if an XBlock read field data in its constructor (forbidden), or if an XBlock was loaded via - # some means other than runtime.get_block(). One way this can happen is if you log/print an XBlock during - # XML parsing, because ScopedStorageMixin.__repr__ will try to print all field values, and any fields which - # aren't mentioned in the XML (which are left at their default) will be "not loaded yet." + # some means other than runtime.get_block() log.exception( "XBlock %s tried to read from field data (%s) that wasn't loaded from Blockstore!", block.scope_ids.usage_id, name, diff --git a/openedx/core/djangoapps/xblock/runtime/blockstore_runtime.py b/openedx/core/djangoapps/xblock/runtime/blockstore_runtime.py index 2f474ebdd2..8b3558bced 100644 --- a/openedx/core/djangoapps/xblock/runtime/blockstore_runtime.py +++ b/openedx/core/djangoapps/xblock/runtime/blockstore_runtime.py @@ -13,7 +13,7 @@ from xblock.fields import ScopeIds from openedx.core.djangoapps.xblock.learning_context.manager import get_learning_context_impl from openedx.core.djangoapps.xblock.runtime.runtime import XBlockRuntime -from openedx.core.djangoapps.xblock.runtime.olx_parsing import parse_xblock_include, BundleFormatException +from openedx.core.djangoapps.xblock.runtime.olx_parsing import parse_xblock_include from openedx.core.djangoapps.xblock.runtime.serializer import serialize_xblock from openedx.core.djangolib.blockstore_cache import ( BundleCache, @@ -85,12 +85,7 @@ class BlockstoreXBlockRuntime(XBlockRuntime): This runtime API should normally be used via runtime.get_block() -> block.parse_xml() -> runtime.add_node_as_child """ - try: - parsed_include = parse_xblock_include(node) - except BundleFormatException: - # We need to log the XBlock ID or this will be hard to debug - log.error("BundleFormatException when parsing XBlock %s", block.scope_ids.usage_id) - raise # Also log details and stack trace + parsed_include = parse_xblock_include(node) self.add_child_include(block, parsed_include) def add_child_include(self, block, parsed_include): diff --git a/openedx/core/djangoapps/xblock/runtime/olx_parsing.py b/openedx/core/djangoapps/xblock/runtime/olx_parsing.py index 9318cf415a..e4a27e996b 100644 --- a/openedx/core/djangoapps/xblock/runtime/olx_parsing.py +++ b/openedx/core/djangoapps/xblock/runtime/olx_parsing.py @@ -27,9 +27,6 @@ def parse_xblock_include(include_node): # An XBlock include looks like: # # Where "source" and "usage" are optional. - if include_node.tag != 'xblock-include': - # xss-lint: disable=python-wrap-html - raise BundleFormatException("Expected an XML node, but got <{}>".format(include_node.tag)) try: definition_path = include_node.attrib['definition'] except KeyError: diff --git a/openedx/core/djangolib/blockstore_cache.py b/openedx/core/djangolib/blockstore_cache.py index c7b94f2061..c1d12d99c3 100644 --- a/openedx/core/djangolib/blockstore_cache.py +++ b/openedx/core/djangolib/blockstore_cache.py @@ -13,6 +13,7 @@ from datetime import datetime from uuid import UUID from django.core.cache import caches, InvalidCacheBackendError +from django.utils.lru_cache import lru_cache from pytz import UTC import requests @@ -32,7 +33,7 @@ except InvalidCacheBackendError: # (Note that we do usually explicitly invalidate this cache during write # operations though, so this setting mostly affects actions by external systems # on Blockstore or bugs where we left out the cache invalidation step.) -MAX_BLOCKSTORE_CACHE_DELAY = 60 * 5 +MAX_BLOCKSTORE_CACHE_DELAY = 60 class BundleCache(object): @@ -143,19 +144,18 @@ def get_bundle_version_number(bundle_uuid, draft_name=None): return version +@lru_cache(maxsize=200) def get_bundle_version_files_cached(bundle_uuid, bundle_version): """ Get the files in the specified BundleVersion. Since BundleVersions are - immutable, this should be cached as aggressively as possible. + immutable, this should be cached as aggressively as possible (ideally + it would be infinitely but we don't want to use up all available memory). + So this method uses lru_cache() to cache results in process-local memory. + + (Note: This can't use BundleCache because BundleCache only associates data + with the most recent bundleversion, not a specified bundleversion) """ - # Use the blockstore django cache directly; this can't use BundleCache because BundleCache only associates data - # with the most recent bundleversion, not a specified bundleversion - cache_key = 'bundle_version_files:{}:{}'.format(bundle_uuid, bundle_version) - result = cache.get(cache_key) - if result is None: - result = blockstore_api.get_bundle_version_files(bundle_uuid, bundle_version) - cache.set(cache_key, result, timeout=None) # Cache forever since bundle versions are immutable - return result + return blockstore_api.get_bundle_version_files(bundle_uuid, bundle_version) def get_bundle_draft_files_cached(bundle_uuid, draft_name): @@ -202,34 +202,24 @@ def get_bundle_file_data_with_cache(bundle_uuid, path, bundle_version=None, draf cached list of files in each bundle if available. """ file_info = get_bundle_file_metadata_with_cache(bundle_uuid, path, bundle_version, draft_name) - response = requests.get(file_info.url) - if response.status_code != 200: - try: - error_response = response.content.decode('utf-8')[:500] - except UnicodeDecodeError: - error_response = '(error details unavailable - response was not a [unicode] string)' - raise blockstore_api.BundleStorageError( - "Unexpected error trying to read {} from bundle {}: \n{}".format(path, bundle_uuid, error_response) - ) - return response.content + with requests.get(file_info.url, stream=True) as r: + return r.content +@lru_cache(maxsize=200) def get_bundle_version_direct_links_cached(bundle_uuid, bundle_version): """ Get the direct links in the specified BundleVersion. Since BundleVersions - are immutable, this should be cached as aggressively as possible. + are immutable, this should be cached as aggressively as possible (ideally + it would be infinitely but we don't want to use up all available memory). + So this method uses lru_cache() to cache results in process-local memory. + + (Note: This can't use BundleCache because BundleCache only associates data + with the most recent bundleversion, not a specified bundleversion) """ - # Use the blockstore django cache directly; this can't use BundleCache because BundleCache only associates data - # with the most recent bundleversion, not a specified bundleversion - cache_key = 'bundle_version_direct_links:{}:{}'.format(bundle_uuid, bundle_version) - result = cache.get(cache_key) - if result is None: - result = { - link.name: link.direct - for link in blockstore_api.get_bundle_version_links(bundle_uuid, bundle_version).values() - } - cache.set(cache_key, result, timeout=None) # Cache forever since bundle versions are immutable - return result + return { + link.name: link.direct for link in blockstore_api.get_bundle_version_links(bundle_uuid, bundle_version).values() + } def get_bundle_draft_direct_links_cached(bundle_uuid, draft_name): diff --git a/openedx/core/lib/blockstore_api/__init__.py b/openedx/core/lib/blockstore_api/__init__.py index 8b6de205c6..288aeada6c 100644 --- a/openedx/core/lib/blockstore_api/__init__.py +++ b/openedx/core/lib/blockstore_api/__init__.py @@ -51,5 +51,4 @@ from .exceptions import ( BundleNotFound, DraftNotFound, BundleFileNotFound, - BundleStorageError, ) diff --git a/openedx/core/lib/blockstore_api/exceptions.py b/openedx/core/lib/blockstore_api/exceptions.py index b58251d31f..06e6de93c6 100644 --- a/openedx/core/lib/blockstore_api/exceptions.py +++ b/openedx/core/lib/blockstore_api/exceptions.py @@ -25,7 +25,3 @@ class DraftNotFound(NotFound): class BundleFileNotFound(NotFound): pass - - -class BundleStorageError(BlockstoreException): - pass