From 6057527689392fe79bfbd5d9e109c1b2490230ee Mon Sep 17 00:00:00 2001 From: Brendan Date: Tue, 7 May 2019 12:13:02 -0400 Subject: [PATCH] =?UTF-8?q?INCR-227:=20python-modernize=20on=20common/lib/?= =?UTF-8?q?xmodule/xmodule/video=5Fmodule=E2=80=A6=20(#20427)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * INCR-227: python-modernize on common/lib/xmodule/xmodule/video_module, partitions, and util * pylint will ignore imports from six * INCR-227 Fixing mangled comments and variable names --- .../xmodule/xmodule/partitions/partitions.py | 2 + .../xmodule/partitions/partitions_service.py | 17 ++++--- .../partitions/tests/test_partitions.py | 24 ++++++---- common/lib/xmodule/xmodule/util/duedate.py | 2 + common/lib/xmodule/xmodule/util/misc.py | 2 + common/lib/xmodule/xmodule/util/sandboxing.py | 6 ++- .../xmodule/xmodule/util/xmodule_django.py | 4 +- .../xmodule/xmodule/video_module/__init__.py | 6 +-- .../xmodule/video_module/bumper_utils.py | 10 +++-- .../xmodule/video_module/transcripts_utils.py | 37 +++++++++------- .../xmodule/video_module/video_handlers.py | 23 +++++----- .../xmodule/video_module/video_module.py | 44 ++++++++++--------- .../xmodule/video_module/video_utils.py | 15 ++++--- .../xmodule/video_module/video_xfields.py | 5 ++- 14 files changed, 115 insertions(+), 82 deletions(-) diff --git a/common/lib/xmodule/xmodule/partitions/partitions.py b/common/lib/xmodule/xmodule/partitions/partitions.py index 9c62c957ac..a0b34f8b94 100644 --- a/common/lib/xmodule/xmodule/partitions/partitions.py +++ b/common/lib/xmodule/xmodule/partitions/partitions.py @@ -1,5 +1,7 @@ """Defines ``Group`` and ``UserPartition`` models for partitioning""" +from __future__ import absolute_import + from collections import namedtuple from stevedore.extension import ExtensionManager diff --git a/common/lib/xmodule/xmodule/partitions/partitions_service.py b/common/lib/xmodule/xmodule/partitions/partitions_service.py index 3fb0d8740d..0bcea9f7e2 100644 --- a/common/lib/xmodule/xmodule/partitions/partitions_service.py +++ b/common/lib/xmodule/xmodule/partitions/partitions_service.py @@ -3,19 +3,18 @@ This is a service-like API that assigns tracks which groups users are in for var user partitions. It uses the user_service key/value store provided by the LMS runtime to persist the assignments. """ +from __future__ import absolute_import + +import logging + +import six from django.conf import settings from django.utils.translation import ugettext_lazy as _ -import logging from openedx.core.lib.cache_utils import request_cached from openedx.features.content_type_gating.partitions import create_content_gating_partition -from xmodule.partitions.partitions import ( - UserPartition, - UserPartitionError, - ENROLLMENT_TRACK_PARTITION_ID, -) from xmodule.modulestore.django import modulestore - +from xmodule.partitions.partitions import ENROLLMENT_TRACK_PARTITION_ID, UserPartition, UserPartitionError log = logging.getLogger(__name__) @@ -105,7 +104,7 @@ def _create_enrollment_track_partition(course): "Can't add 'enrollment_track' partition, as ID {id} is assigned to {partition} in course {course}.".format( id=ENROLLMENT_TRACK_PARTITION_ID, partition=get_partition_from_id(course.user_partitions, ENROLLMENT_TRACK_PARTITION_ID).name, - course=unicode(course.id) + course=six.text_type(course.id) ) ) return None @@ -114,7 +113,7 @@ def _create_enrollment_track_partition(course): id=ENROLLMENT_TRACK_PARTITION_ID, name=_(u"Enrollment Track Groups"), description=_(u"Partition for segmenting users by enrollment track"), - parameters={"course_id": unicode(course.id)} + parameters={"course_id": six.text_type(course.id)} ) return partition diff --git a/common/lib/xmodule/xmodule/partitions/tests/test_partitions.py b/common/lib/xmodule/xmodule/partitions/tests/test_partitions.py index 8417a16faf..5418c7dd4d 100644 --- a/common/lib/xmodule/xmodule/partitions/tests/test_partitions.py +++ b/common/lib/xmodule/xmodule/partitions/tests/test_partitions.py @@ -3,20 +3,26 @@ Test the partitions and partitions service """ +from __future__ import absolute_import + from datetime import datetime + +import six from django.test import TestCase from mock import Mock, patch - from opaque_keys.edx.locator import CourseLocator from stevedore.extension import Extension, ExtensionManager -from xmodule.partitions.partitions import ( - Group, UserPartition, UserPartitionError, NoSuchUserPartitionGroupError, - USER_PARTITION_SCHEME_NAMESPACE, ENROLLMENT_TRACK_PARTITION_ID -) -from xmodule.partitions.partitions_service import ( - PartitionService, get_all_partitions_for_course, FEATURES -) + from openedx.features.content_type_gating.models import ContentTypeGatingConfig +from xmodule.partitions.partitions import ( + ENROLLMENT_TRACK_PARTITION_ID, + USER_PARTITION_SCHEME_NAMESPACE, + Group, + NoSuchUserPartitionGroupError, + UserPartition, + UserPartitionError +) +from xmodule.partitions.partitions_service import FEATURES, PartitionService, get_all_partitions_for_course class TestGroup(TestCase): @@ -574,7 +580,7 @@ class TestGetCourseUserPartitions(PartitionServiceBaseClass): self.assertEqual(self.TEST_SCHEME_NAME, all_partitions[0].scheme.name) enrollment_track_partition = all_partitions[1] self.assertEqual(self.ENROLLMENT_TRACK_SCHEME_NAME, enrollment_track_partition.scheme.name) - self.assertEqual(unicode(self.course.id), enrollment_track_partition.parameters['course_id']) + self.assertEqual(six.text_type(self.course.id), enrollment_track_partition.parameters['course_id']) self.assertEqual(ENROLLMENT_TRACK_PARTITION_ID, enrollment_track_partition.id) def test_enrollment_track_partition_not_added_if_conflict(self): diff --git a/common/lib/xmodule/xmodule/util/duedate.py b/common/lib/xmodule/xmodule/util/duedate.py index 7c5dec5eaa..fa42c9b64f 100644 --- a/common/lib/xmodule/xmodule/util/duedate.py +++ b/common/lib/xmodule/xmodule/util/duedate.py @@ -1,6 +1,8 @@ """ Miscellaneous utility functions. """ +from __future__ import absolute_import + from functools import partial diff --git a/common/lib/xmodule/xmodule/util/misc.py b/common/lib/xmodule/xmodule/util/misc.py index 3f5ba4aa2c..daa6074f1e 100644 --- a/common/lib/xmodule/xmodule/util/misc.py +++ b/common/lib/xmodule/xmodule/util/misc.py @@ -1,6 +1,8 @@ """ Miscellaneous utility functions. """ +from __future__ import absolute_import + import re from xmodule.annotator_mixin import html_to_text diff --git a/common/lib/xmodule/xmodule/util/sandboxing.py b/common/lib/xmodule/xmodule/util/sandboxing.py index b68fb75b8b..e2def9c918 100644 --- a/common/lib/xmodule/xmodule/util/sandboxing.py +++ b/common/lib/xmodule/xmodule/util/sandboxing.py @@ -1,4 +1,8 @@ +from __future__ import absolute_import + import re + +import six from django.conf import settings DEFAULT_PYTHON_LIB_FILENAME = 'python_lib.zip' @@ -24,7 +28,7 @@ def can_execute_unsafe_code(course_id): # To others using this: the code as-is is brittle and likely to be changed in the future, # as per the TODO, so please consider carefully before adding more values to COURSES_WITH_UNSAFE_CODE for regex in getattr(settings, 'COURSES_WITH_UNSAFE_CODE', []): - if re.match(regex, unicode(course_id)): + if re.match(regex, six.text_type(course_id)): return True return False diff --git a/common/lib/xmodule/xmodule/util/xmodule_django.py b/common/lib/xmodule/xmodule/util/xmodule_django.py index 44c9b449df..0fc114b0ab 100644 --- a/common/lib/xmodule/xmodule/util/xmodule_django.py +++ b/common/lib/xmodule/xmodule/util/xmodule_django.py @@ -4,9 +4,11 @@ NOTE: This file should only be imported into 'django-safe' code, i.e. known that runtime environment with the djangoapps in common configured to load """ +from __future__ import absolute_import + +import webpack_loader # NOTE: we are importing this method so that any module that imports us has access to get_current_request from crum import get_current_request -import webpack_loader def get_current_request_hostname(): diff --git a/common/lib/xmodule/xmodule/video_module/__init__.py b/common/lib/xmodule/xmodule/video_module/__init__.py index 2c9ac1ae9c..cbee7351b9 100644 --- a/common/lib/xmodule/xmodule/video_module/__init__.py +++ b/common/lib/xmodule/xmodule/video_module/__init__.py @@ -4,7 +4,7 @@ Container for video module and its utils. # pylint: disable=wildcard-import -from .transcripts_utils import * -from .video_utils import * -from .video_module import * from .bumper_utils import * +from .transcripts_utils import * +from .video_module import * +from .video_utils import * diff --git a/common/lib/xmodule/xmodule/video_module/bumper_utils.py b/common/lib/xmodule/xmodule/video_module/bumper_utils.py index dce5b7d80b..c440b6ace3 100644 --- a/common/lib/xmodule/xmodule/video_module/bumper_utils.py +++ b/common/lib/xmodule/xmodule/video_module/bumper_utils.py @@ -1,14 +1,16 @@ """ Utils for video bumper """ -from collections import OrderedDict +from __future__ import absolute_import + import copy import json import logging - +from collections import OrderedDict from datetime import datetime, timedelta -from django.conf import settings + import pytz +from django.conf import settings from .video_utils import set_query_parameter @@ -106,7 +108,7 @@ def get_bumper_sources(video): try: val_profiles = ["desktop_webm", "desktop_mp4"] val_video_urls = edxval_api.get_urls_for_profiles(video.bumper['edx_video_id'], val_profiles) - bumper_sources = filter(None, [val_video_urls[p] for p in val_profiles]) + bumper_sources = [url for url in [val_video_urls[p] for p in val_profiles] if url] except edxval_api.ValInternalError: # if no bumper sources, nothing will be showed log.warning( diff --git a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py index 28013bfb39..0ae8e3e98c 100644 --- a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py +++ b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py @@ -2,22 +2,27 @@ Utility functions for transcripts. ++++++++++++++++++++++++++++++++++ """ -from functools import wraps -from django.conf import settings -import os +from __future__ import absolute_import + import copy import json -import requests import logging -from pysrt import SubRipTime, SubRipItem, SubRipFile -from pysrt.srtexc import Error -from lxml import etree -from HTMLParser import HTMLParser -from six import text_type +import os +from functools import wraps + +import requests +import six +from django.conf import settings +from lxml import etree +from pysrt import SubRipFile, SubRipItem, SubRipTime +from pysrt.srtexc import Error +from six import text_type +from six.moves import range, zip +from six.moves.html_parser import HTMLParser # pylint: disable=import-error -from xmodule.exceptions import NotFoundError from xmodule.contentstore.content import StaticContent from xmodule.contentstore.django import contentstore +from xmodule.exceptions import NotFoundError from .bumper_utils import get_bumper_settings @@ -266,7 +271,7 @@ def generate_subs_from_source(speed_subs, subs_type, subs_filedata, item, langua 'end': sub_ends, 'text': sub_texts} - for speed, subs_id in speed_subs.iteritems(): + for speed, subs_id in six.iteritems(speed_subs): save_subs_to_store( generate_subs(speed, 1, subs), subs_id, @@ -300,7 +305,7 @@ def generate_srt_from_sjson(sjson_subs, speed): end=SubRipTime(milliseconds=sjson_speed_1['end'][i]), text=sjson_speed_1['text'][i] ) - output += (unicode(item)) + output += (six.text_type(item)) output += '\n' return output @@ -439,7 +444,7 @@ def manage_video_subtitles_save(item, user, old_metadata=None, generate_translat generate_sjson_for_all_speeds( item, item.transcripts[lang], - {speed: subs_id for subs_id, speed in youtube_speed_dict(item).iteritems()}, + {speed: subs_id for subs_id, speed in six.iteritems(youtube_speed_dict(item))}, lang, ) except TranscriptException as ex: @@ -537,13 +542,13 @@ def get_video_ids_info(edx_video_id, youtube_id_1_0, html5_sources): Returns: tuple: external or internal, video ids list """ - clean = lambda item: item.strip() if isinstance(item, basestring) else item + clean = lambda item: item.strip() if isinstance(item, six.string_types) else item external = not bool(clean(edx_video_id)) video_ids = [edx_video_id, youtube_id_1_0] + get_html5_ids(html5_sources) # video_ids cleanup - video_ids = filter(lambda item: bool(clean(item)), video_ids) + video_ids = [item for item in video_ids if bool(clean(item))] return external, video_ids @@ -755,7 +760,7 @@ class VideoTranscriptsMixin(object): if sub: all_langs.update({'en': sub}) - for language, filename in all_langs.iteritems(): + for language, filename in six.iteritems(all_langs): try: # for bumper videos, transcripts are stored in content store only if is_bumper: diff --git a/common/lib/xmodule/xmodule/video_module/video_handlers.py b/common/lib/xmodule/xmodule/video_module/video_handlers.py index 837f8625bc..6c4106f609 100644 --- a/common/lib/xmodule/xmodule/video_module/video_handlers.py +++ b/common/lib/xmodule/xmodule/video_module/video_handlers.py @@ -5,35 +5,36 @@ StudentViewHandlers are handlers for video module instance. StudioViewHandlers are handlers for video descriptor instance. """ +from __future__ import absolute_import + import json import logging import six from django.core.files.base import ContentFile from django.utils.timezone import now +from edxval.api import create_external_video, create_or_update_video_transcript, delete_video_transcript +from opaque_keys.edx.locator import CourseLocator from webob import Response - from xblock.core import XBlock from xblock.exceptions import JsonHandlerError from xmodule.exceptions import NotFoundError from xmodule.fields import RelativeTime -from opaque_keys.edx.locator import CourseLocator -from edxval.api import create_or_update_video_transcript, create_external_video, delete_video_transcript from .transcripts_utils import ( - clean_video_id, - get_or_create_sjson, - generate_sjson_for_all_speeds, - subs_filename, Transcript, TranscriptException, TranscriptsGenerationException, - youtube_speed_dict, + clean_video_id, + generate_sjson_for_all_speeds, + get_html5_ids, + get_or_create_sjson, get_transcript, get_transcript_from_contentstore, remove_subs_from_store, - get_html5_ids + subs_filename, + youtube_speed_dict ) log = logging.getLogger(__name__) @@ -153,7 +154,7 @@ class VideoStudentViewHandlers(object): generate_sjson_for_all_speeds( self, other_lang[self.transcript_language], - {speed: youtube_id for youtube_id, speed in youtube_ids.iteritems()}, + {speed: youtube_id for youtube_id, speed in six.iteritems(youtube_ids)}, self.transcript_language ) sjson_transcript = Transcript.asset(self.location, youtube_id, self.transcript_language).data @@ -309,7 +310,7 @@ class VideoStudentViewHandlers(object): log.info("Invalid /translation request: no language.") return Response(status=400) - if language not in ['en'] + transcripts["transcripts"].keys(): + if language not in ['en'] + list(transcripts["transcripts"].keys()): log.info("Video: transcript facilities are not available for given language.") return Response(status=404) diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py index ad8a7533c4..d83bab3869 100644 --- a/common/lib/xmodule/xmodule/video_module/video_module.py +++ b/common/lib/xmodule/xmodule/video_module/video_module.py @@ -12,25 +12,29 @@ Examples of html5 videos for manual testing: https://s3.amazonaws.com/edx-course-videos/edx-intro/edX-FA12-cware-1_100.webm https://s3.amazonaws.com/edx-course-videos/edx-intro/edX-FA12-cware-1_100.ogv """ +from __future__ import absolute_import + import copy import json import logging -from collections import defaultdict, OrderedDict +from collections import OrderedDict, defaultdict from operator import itemgetter -from pkg_resources import resource_string - +import six from django.conf import settings from lxml import etree from opaque_keys.edx.locator import AssetLocator -from openedx.core.djangoapps.video_config.models import HLSPlaybackEnabledFlag -from openedx.core.djangoapps.video_pipeline.config.waffle import waffle_flags, DEPRECATE_YOUTUBE -from openedx.core.lib.cache_utils import request_cached -from openedx.core.lib.license import LicenseMixin +from pkg_resources import resource_string +from web_fragments.fragment import Fragment from xblock.completable import XBlockCompletionMode from xblock.core import XBlock from xblock.fields import ScopeIds from xblock.runtime import KvsFieldData + +from openedx.core.djangoapps.video_config.models import HLSPlaybackEnabledFlag +from openedx.core.djangoapps.video_pipeline.config.waffle import DEPRECATE_YOUTUBE, waffle_flags +from openedx.core.lib.cache_utils import request_cached +from openedx.core.lib.license import LicenseMixin from xmodule.contentstore.content import StaticContent from xmodule.editing_module import TabsEditingDescriptor from xmodule.exceptions import NotFoundError @@ -38,23 +42,21 @@ from xmodule.modulestore.inheritance import InheritanceKeyValueStore, own_metada from xmodule.raw_module import EmptyDataRawDescriptor from xmodule.validation import StudioValidation, StudioValidationMessage from xmodule.video_module import manage_video_subtitles_save -from xmodule.x_module import XModule, module_attr, PUBLIC_VIEW, STUDENT_VIEW +from xmodule.x_module import PUBLIC_VIEW, STUDENT_VIEW, XModule, module_attr from xmodule.xml_module import deserialize_field, is_pointer_tag, name_to_pathname from .bumper_utils import bumperize from .transcripts_utils import ( - get_html5_ids, Transcript, VideoTranscriptsMixin, clean_video_id, - subs_filename, - get_transcript_for_video + get_html5_ids, + get_transcript_for_video, + subs_filename ) - from .video_handlers import VideoStudentViewHandlers, VideoStudioViewHandlers from .video_utils import create_youtube_string, format_xml_exception_message, get_poster, rewrite_video_url from .video_xfields import VideoFields -from web_fragments.fragment import Fragment # The following import/except block for edxval is temporary measure until # edxval is a proper XBlock Runtime Service. @@ -176,7 +178,7 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers, languages['en'] = 'English' # OrderedDict for easy testing of rendered context in tests - sorted_languages = sorted(languages.items(), key=itemgetter(1)) + sorted_languages = sorted(list(languages.items()), key=itemgetter(1)) sorted_languages = OrderedDict(sorted_languages) return track_url, transcript_language, sorted_languages @@ -219,7 +221,7 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers, track_status = (self.download_track and self.track) transcript_download_format = self.transcript_download_format if not track_status else None - sources = filter(None, self.html5_sources) + sources = [source for source in self.html5_sources if source] download_video_link = None branding_info = None @@ -687,7 +689,7 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler # Mild workaround to ensure that tests pass -- if a field # is set to its default value, we don't need to write it out. if youtube_string and youtube_string != '1.00:3_yD_cEKoCk': - xml.set('youtube', unicode(youtube_string)) + xml.set('youtube', six.text_type(youtube_string)) xml.set('url_name', self.url_name) attrs = { 'display_name': self.display_name, @@ -704,13 +706,13 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler if value: if key in self.fields and self.fields[key].is_set_on(self): try: - xml.set(key, unicode(value)) + xml.set(key, six.text_type(value)) except UnicodeDecodeError: exception_message = format_xml_exception_message(self.location, key, value) log.exception(exception_message) # If exception is UnicodeDecodeError set value using unicode 'utf-8' scheme. log.info("Setting xml value using 'utf-8' scheme.") - xml.set(key, unicode(value, 'utf-8')) + xml.set(key, six.text_type(value, 'utf-8')) except ValueError: exception_message = format_xml_exception_message(self.location, key, value) log.exception(exception_message) @@ -752,7 +754,7 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler video_id=edx_video_id, resource_fs=resource_fs, static_dir=EXPORT_IMPORT_STATIC_DIR, - course_id=unicode(self.runtime.course_id.for_branch(None)) + course_id=six.text_type(self.runtime.course_id.for_branch(None)) ) # Update xml with edxval metadata xml.append(exported_metadata['xml']) @@ -957,7 +959,7 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler course_id = getattr(id_generator, 'target_course_id', None) # Update the handout location with current course_id - if 'handout' in field_data.keys() and course_id: + if 'handout' in list(field_data.keys()) and course_id: handout_location = StaticContent.get_location_from_path(field_data['handout']) if isinstance(handout_location, AssetLocator): handout_new_location = StaticContent.compute_location(course_id, handout_location.path) @@ -1072,7 +1074,7 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler """ Returns the VAL data for the requested video profiles for the given course. """ - return edxval_api.get_video_info_for_course_and_profiles(unicode(course_id), video_profile_names) + return edxval_api.get_video_info_for_course_and_profiles(six.text_type(course_id), video_profile_names) def student_view_data(self, context=None): """ diff --git a/common/lib/xmodule/xmodule/video_module/video_utils.py b/common/lib/xmodule/xmodule/video_module/video_utils.py index 1c24303ada..2091fe480b 100644 --- a/common/lib/xmodule/xmodule/video_module/video_utils.py +++ b/common/lib/xmodule/xmodule/video_module/video_utils.py @@ -3,14 +3,17 @@ Module contains utils specific for video_module but not for transcripts. """ -from collections import OrderedDict -import logging -from urllib import urlencode -from urlparse import parse_qs, urlsplit, urlunsplit, urlparse +from __future__ import absolute_import +import logging +from collections import OrderedDict + +import six from django.conf import settings -from django.core.validators import URLValidator from django.core.exceptions import ValidationError +from django.core.validators import URLValidator +from six.moves import zip +from six.moves.urllib.parse import parse_qs, urlencode, urlparse, urlsplit, urlunsplit # pylint: disable=import-error log = logging.getLogger(__name__) @@ -104,7 +107,7 @@ def format_xml_exception_message(location, key, value): when setting xml attributes. """ exception_message = "Block-location:{location}, Key:{key}, Value:{value}".format( - location=unicode(location), + location=six.text_type(location), key=key, value=value ) diff --git a/common/lib/xmodule/xmodule/video_module/video_xfields.py b/common/lib/xmodule/xmodule/video_module/video_xfields.py index 0eff8fa488..da39b0c11f 100644 --- a/common/lib/xmodule/xmodule/video_module/video_xfields.py +++ b/common/lib/xmodule/xmodule/video_module/video_xfields.py @@ -1,9 +1,12 @@ """ XFields for video module. """ +from __future__ import absolute_import + import datetime -from xblock.fields import Scope, String, Float, Boolean, List, Dict, DateTime +from xblock.fields import Boolean, DateTime, Dict, Float, List, Scope, String + from xmodule.fields import RelativeTime # Make '_' a no-op so we can scrape strings. Using lambda instead of