Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: [AXM-791] fix error 404 handling and optimize for not modified b… #2582

Merged
Show file tree
Hide file tree
Changes from 1 commit
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 cms/djangoapps/contentstore/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 17 additions & 9 deletions openedx/features/offline_mode/storage_management.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

from .assets_management import block_storage_path, clean_outdated_xblock_files, is_modified
from .html_manipulator import HtmlManipulator
from .renderer import XBlockRenderer

User = get_user_model()
log = logging.getLogger(__name__)
Expand All @@ -25,28 +26,40 @@ 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.
"""
if storage_kwargs is 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:
log.error(
f'Block {str(self.xblock.location)} cannot be fetched from course'
f' {self.xblock.location.course_key} during offline content generation.'
)
return None

def generate_offline_content(self):
"""
Generates archive with XBlock content for offline mode.
"""
if not is_modified(self.xblock):
if not self.html_data:
return

base_path = block_storage_path(self.xblock)
Expand All @@ -56,11 +69,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)

Expand Down
13 changes: 6 additions & 7 deletions openedx/features/offline_mode/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -24,10 +24,9 @@ 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(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we don't need retry for generic Exception?

Expand All @@ -36,10 +35,10 @@ def generate_offline_content_for_course(course_id):
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()
Loading