From 3e076fcf0466e31fc2d3056f4a09b676fb4fe757 Mon Sep 17 00:00:00 2001 From: Ivan Niedielnitsev <81557788+NiedielnitsevIvan@users.noreply.github.com> Date: Fri, 21 Jun 2024 11:05:07 +0300 Subject: [PATCH] =?UTF-8?q?fix:=20[AXM-791]=20fix=20error=20404=20handling?= =?UTF-8?q?=20and=20optimize=20for=20not=20modified=20b=E2=80=A6=20(#2582)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: [AXM-791] fix error 404 handling and optimize for not modified blocks * fix: [AXM-791] remove unsed import and generic Exception from retry --- cms/djangoapps/contentstore/utils.py | 2 +- .../offline_mode/storage_management.py | 29 +++++++++++-------- openedx/features/offline_mode/tasks.py | 15 +++++----- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index c86accc22fb6..8eaf31f927bd 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -2328,7 +2328,7 @@ def get_cms_api_client(): """ Returns an API client which can be used to make requests from the CMS service. """ - user = User.objects.get(username=settings.EDXAPP_CMS_SERVICE_USER_NAME) + user = User.objects.get(username=settings.CMS_SERVICE_USER_NAME) jwt = create_jwt_for_user(user) client = requests.Session() client.auth = SuppliedJwtAuth(jwt) diff --git a/openedx/features/offline_mode/storage_management.py b/openedx/features/offline_mode/storage_management.py index 2811485695b7..92658b68c1af 100644 --- a/openedx/features/offline_mode/storage_management.py +++ b/openedx/features/offline_mode/storage_management.py @@ -13,8 +13,9 @@ from openedx.core.storage import get_storage from zipfile import ZipFile -from .assets_management import block_storage_path, clean_outdated_xblock_files, is_modified +from .assets_management import block_storage_path, clean_outdated_xblock_files from .html_manipulator import HtmlManipulator +from .renderer import XBlockRenderer User = get_user_model() log = logging.getLogger(__name__) @@ -25,13 +26,12 @@ class OfflineContentGenerator: Creates zip file with Offline Content in the media storage. """ - def __init__(self, xblock, html_data, storage_class=None, storage_kwargs=None): + def __init__(self, xblock, html_data=None, storage_class=None, storage_kwargs=None): """ Creates `SaveOfflineContentToStorage` object. Args: xblock (XBlock): The XBlock instance - html_data (str): The rendered HTML representation of the XBlock storage_class: Used media storage class. storage_kwargs (dict): Additional storage attributes. """ @@ -39,16 +39,26 @@ def __init__(self, xblock, html_data, storage_class=None, storage_kwargs=None): storage_kwargs = {} self.xblock = xblock - self.html_data = html_data + self.html_data = html_data or self.render_block_html_data() self.storage = get_storage(storage_class, **storage_kwargs) + def render_block_html_data(self): + """ + Renders the XBlock HTML content from the LMS. + """ + try: + return XBlockRenderer(str(self.xblock.location)).render_xblock_from_lms() + except Http404 as e: + log.error( + f'Block {str(self.xblock.location)} cannot be fetched from course' + f' {self.xblock.location.course_key} during offline content generation.' + ) + raise e + def generate_offline_content(self): """ Generates archive with XBlock content for offline mode. """ - if not is_modified(self.xblock): - return - base_path = block_storage_path(self.xblock) clean_outdated_xblock_files(self.xblock) tmp_dir = mkdtemp() @@ -56,11 +66,6 @@ def generate_offline_content(self): try: self.save_xblock_html(tmp_dir) self.create_zip_file(tmp_dir, base_path, f'{self.xblock.location.block_id}.zip') - except Http404: - log.error( - f'Block {self.xblock.location.block_id} cannot be fetched from course' - f' {self.xblock.location.course_key} during offline content generation.' - ) finally: shutil.rmtree(tmp_dir, ignore_errors=True) diff --git a/openedx/features/offline_mode/tasks.py b/openedx/features/offline_mode/tasks.py index 224840fc07d3..0fe4de0f51cd 100644 --- a/openedx/features/offline_mode/tasks.py +++ b/openedx/features/offline_mode/tasks.py @@ -8,8 +8,8 @@ from xmodule.modulestore.django import modulestore +from .assets_management import is_modified from .constants import MAX_RETRY_ATTEMPTS, OFFLINE_SUPPORTED_XBLOCKS, RETRY_BACKOFF_INITIAL_TIMEOUT -from .renderer import XBlockRenderer from .storage_management import OfflineContentGenerator @@ -24,22 +24,21 @@ def generate_offline_content_for_course(course_id): course_key = CourseKey.from_string(course_id) for offline_supported_block_type in OFFLINE_SUPPORTED_XBLOCKS: for xblock in modulestore().get_items(course_key, qualifiers={'category': offline_supported_block_type}): - if not hasattr(xblock, 'closed') or not xblock.closed(): - block_id = str(xblock.location) - html_data = XBlockRenderer(block_id).render_xblock_from_lms() - generate_offline_content_for_block.apply_async([block_id, html_data]) + is_not_closed = not hasattr(xblock, 'closed') or not xblock.closed() + if is_not_closed and is_modified(xblock): + generate_offline_content_for_block.apply_async([str(xblock.location)]) @shared_task( - autoretry_for=(Exception, Http404), + autoretry_for=(Http404,), retry_backoff=RETRY_BACKOFF_INITIAL_TIMEOUT, retry_kwargs={'max_retries': MAX_RETRY_ATTEMPTS} ) @set_code_owner_attribute -def generate_offline_content_for_block(block_id, html_data): +def generate_offline_content_for_block(block_id): """ Generates offline content for the specified block. """ block_usage_key = UsageKey.from_string(block_id) xblock = modulestore().get_item(block_usage_key) - OfflineContentGenerator(xblock, html_data).generate_offline_content() + OfflineContentGenerator(xblock).generate_offline_content()