Merge pull request #181 from edx/dcs/fix-py3

Fix python3 incompatibilities

edxval/api.py

@@ -1051,7 +1051,7 @@ def import_from_xml(xml, edx_video_id, resource_fs, static_dir, external_transcr
 
         return edx_video_id
     except ValidationError as err:
-        logger.exception(err.message)
+        logger.exception(xml)
         raise ValCannotCreateError(err.message_dict)
     except Video.DoesNotExist:
         pass
@@ -1122,9 +1122,8 @@ def import_transcript_from_fs(edx_video_id, language_code, file_name, provider,
     if not transcript_data:
         # Read file from import file system and attach it to transcript record in DS.
         try:
-            with resource_fs.open(combine(static_dir, file_name), 'rb') as f:
+            with resource_fs.open(combine(static_dir, file_name), 'r', encoding='utf-8-sig') as f:
                 file_content = f.read()
-                file_content = file_content.decode('utf-8-sig')
         except ResourceNotFound as exc:
             # Don't raise exception in case transcript file is not found in course OLX.
             logger.warn(

edxval/tests/constants.py

@@ -3,7 +3,7 @@
 """
 Constants used for tests.
 """
-from __future__ import absolute_import
+from __future__ import absolute_import, unicode_literals
 from edxval.models import (
     TranscriptProviderType,
     Cielo24Fidelity,

edxval/tests/test_api.py

@@ -7,6 +7,8 @@
 import os
 import shutil
 
+from io import open
+
 import mock
 from unittest import skip
 from ddt import data, ddt, unpack
@@ -1131,7 +1133,7 @@ def test_basic(self, course_id, image):
         )
 
         self.assert_xml_equal(exported_metadata['xml'], expected)
-        self.assertItemsEqual(list(exported_metadata['transcripts'].keys()), ['en', 'de'])
+        self.assertEqual(sorted(exported_metadata['transcripts'].keys()), ['de', 'en'])
 
     def test_transcript_export(self):
         """
@@ -1168,12 +1170,12 @@ def test_transcript_export(self):
         self.assert_xml_equal(exported_metadata['xml'], expected_xml)
 
         # Verify transcript file is created.
-        self.assertItemsEqual(list(transcript_files.values()), self.file_system.listdir(constants.EXPORT_IMPORT_STATIC_DIR))
+        self.assertEqual(sorted(transcript_files.values()), sorted(self.file_system.listdir(constants.EXPORT_IMPORT_STATIC_DIR)))
 
         # Also verify the content of created transcript file.
         for language_code in transcript_files.keys():
             expected_transcript_content = File(
-                open(combine(expected_transcript_path, transcript_files[language_code]))
+                open(combine(expected_transcript_path, transcript_files[language_code]), 'rb')
             ).read()
             transcript = api.get_video_transcript_data(video_id=video_id, language_code=language_code)
             transcript_format = os.path.splitext(transcript['file_name'])[1][1:]
@@ -1327,7 +1329,7 @@ def assert_transcripts(self, video_id, expected_transcripts):
             ).data
 
             # Assert transcript content
-            received_transcript['file_data'] = api.get_video_transcript_data(video_id, language_code)['content']
+            received_transcript['file_data'] = api.get_video_transcript_data(video_id, language_code)['content'].decode('utf8')
 
             # Omit not needed attrs.
             expected_transcript = omit_attrs(expected_transcript, ['transcript'])
@@ -2041,12 +2043,12 @@ def test_video_transcript_missing_attribute(self, mock_logger):
         Verify that video transcript import working as expected if transcript xml data is missing.
         """
         video_id = 'super-soaker'
-        transcript_xml = '<transcript file_format="srt" provider="Cielo24"/>'
+        transcript_xml = u'<transcript file_format="srt" provider="Cielo24"/>'
         xml = etree.fromstring("""
             <video_asset>
                 <transcripts>
                     {transcript_xml}
-                    <transcript language_code="de" file_format="sjson" provider='3PlayMedia'/>
+                    <transcript language_code="de" file_format="sjson" provider="3PlayMedia"/>
                 </transcripts>
             </video_asset>
         """.format(transcript_xml=transcript_xml))
@@ -2066,7 +2068,7 @@ def test_video_transcript_missing_attribute(self, mock_logger):
 
         mock_logger.warn.assert_called_with(
             "VAL: Required attributes are missing from xml, xml=[%s]",
-            transcript_xml
+            transcript_xml.encode('utf8')
         )
 
         self.assert_transcripts(video_id, [self.transcript_data3])
@@ -2175,10 +2177,10 @@ def setUp(self):
         self.image_path1 = 'edxval/tests/data/image.jpg'
         self.image_path2 = 'edxval/tests/data/edx.jpg'
         self.image_url = api.update_video_image(
-            self.edx_video_id, self.course_id, ImageFile(open(self.image_path1)), 'image.jpg'
+            self.edx_video_id, self.course_id, ImageFile(open(self.image_path1, 'rb')), 'image.jpg'
         )
         self.image_url2 = api.update_video_image(
-            self.edx_video_id, self.course_id2, ImageFile(open(self.image_path2)), 'image.jpg'
+            self.edx_video_id, self.course_id2, ImageFile(open(self.image_path2, 'rb')), 'image.jpg'
         )
 
     def test_update_video_image(self):
@@ -2187,8 +2189,8 @@ def test_update_video_image(self):
         """
         self.assertEqual(self.course_video.video_image.image.name, self.image_url)
         self.assertEqual(self.course_video2.video_image.image.name, self.image_url2)
-        self.assertEqual(ImageFile(open(self.image_path1)).size, ImageFile(open(self.image_url)).size)
-        self.assertEqual(ImageFile(open(self.image_path2)).size, ImageFile(open(self.image_url2)).size)
+        self.assertEqual(ImageFile(open(self.image_path1, 'rb')).size, ImageFile(open(self.image_url, 'rb')).size)
+        self.assertEqual(ImageFile(open(self.image_path2, 'rb')).size, ImageFile(open(self.image_url2, 'rb')).size)
 
     def test_get_course_video_image_url(self):
         """
@@ -2211,7 +2213,7 @@ def test_num_queries_update_video_image(self):
         """
         with self.assertNumQueries(6):
             api.update_video_image(
-                self.edx_video_id, self.course_id, ImageFile(open(self.image_path1)), 'image.jpg'
+                self.edx_video_id, self.course_id, ImageFile(open(self.image_path1, 'rb')), 'image.jpg'
             )
 
     def test_num_queries_get_course_video_image_url(self):
@@ -2245,7 +2247,7 @@ def test_create_or_update_logging(self, mock_logger, mock_image_save):
         Tests correct message is logged when save to storge is failed in `create_or_update`.
         """
         with self.assertRaises(Exception) as save_exception:  # pylint: disable=unused-variable
-            VideoImage.create_or_update(self.course_video, 'test.jpg', open(self.image_path2))
+            VideoImage.create_or_update(self.course_video, 'test.jpg', open(self.image_path2, 'rb'))
 
         mock_logger.exception.assert_called_with(
             'VAL: Video Image save failed to storage for course_id [%s] and video_id [%s]',
@@ -2260,10 +2262,10 @@ def test_update_video_image_exception(self):
         does_not_course_id = 'does_not_exist'
 
         with self.assertRaises(Exception) as get_exception:
-            api.update_video_image(self.edx_video_id, does_not_course_id, open(self.image_path2), 'test.jpg')
+            api.update_video_image(self.edx_video_id, does_not_course_id, open(self.image_path2, 'rb'), 'test.jpg')
 
         self.assertEqual(
-            get_exception.exception.message,
+            get_exception.exception.args[0],
             u'VAL: CourseVideo not found for edx_video_id: {0} and course_id: {1}'.format(
                 self.edx_video_id,
                 does_not_course_id
@@ -2355,7 +2357,7 @@ def test_video_image_deletion_single(self):
 
         # This will replace the image for self.course_video and delete the existing image
         image_url = api.update_video_image(
-            self.edx_video_id, self.course_id, ImageFile(open(self.image_path2)), 'image.jpg'
+            self.edx_video_id, self.course_id, ImageFile(open(self.image_path2, 'rb')), 'image.jpg'
         )
 
         # Verify that new image is set to course_video
@@ -2364,7 +2366,7 @@ def test_video_image_deletion_single(self):
 
         # Verify that an exception is raised if we try to open a delete image file
         with self.assertRaises(IOError) as file_open_exception:
-            ImageFile(open(existing_image_name))
+            ImageFile(open(existing_image_name, 'rb'))
 
         self.assertEqual(file_open_exception.exception.strerror, u'No such file or directory')
 
@@ -2379,7 +2381,7 @@ def test_video_image_deletion_multiple(self):
 
         # This will replace the image for self.course_video but image will
         # not be deleted because it is also used by self.course_video2
-        api.update_video_image(self.edx_video_id, self.course_id, ImageFile(open(self.image_path2)), 'image.jpg')
+        api.update_video_image(self.edx_video_id, self.course_id, ImageFile(open(self.image_path2, 'rb')), 'image.jpg')
 
         # Verify image for course_video has changed
         course_video = CourseVideo.objects.get(video=self.video, course_id=self.course_id)
@@ -2389,7 +2391,7 @@ def test_video_image_deletion_multiple(self):
         self.assertEqual(self.course_video2.video_image.image.name, shared_image)
 
         # Open the shared image file to verify it is not deleted
-        ImageFile(open(shared_image))
+        ImageFile(open(shared_image, 'rb'))
 
 
 @ddt
@@ -2413,7 +2415,7 @@ def setUp(self):
                     'provider': TranscriptProviderType.THREE_PLAY_MEDIA,
                     'file_name': None,
                     'file_format': utils.TranscriptFormat.SRT,
-                    'file_data': File(open(self.flash_transcript_path))
+                    'file_data': File(open(self.flash_transcript_path, 'rb'))
                 },
                 {
                     'language_code': 'fr',
@@ -2437,7 +2439,7 @@ def setUp(self):
                     'provider': TranscriptProviderType.CUSTOM,
                     'file_name': None,
                     'file_format': utils.TranscriptFormat.SRT,
-                    'file_data': File(open(self.arrow_transcript_path))
+                    'file_data': File(open(self.arrow_transcript_path, 'rb'))
                 },
                 {
                     'language_code': 'zh',
@@ -2560,7 +2562,7 @@ def test_get_video_transcript_data(self, video_id, language_code, expected_file_
         """
         expected_transcript = {
             'file_name': expected_file_name,
-            'content': File(open(expected_transcript_path)).read()
+            'content': File(open(expected_transcript_path, 'rb')).read()
         }
         transcript = api.get_video_transcript_data(video_id=video_id, language_code=language_code)
         self.assertDictEqual(transcript, expected_transcript)
@@ -2623,7 +2625,7 @@ def test_create_or_update_video_transcript(self, file_data, file_name, file_form
         if file_data:
             self.assertTrue(transcript_url.startswith(settings.VIDEO_TRANSCRIPTS_SETTINGS['DIRECTORY_PREFIX']))
             self.assertEqual(video_transcript.transcript.name, transcript_url)
-            with open(video_transcript.transcript.name) as saved_transcript:
+            with open(video_transcript.transcript.name, encoding='utf8') as saved_transcript:
                 self.assertEqual(saved_transcript.read(), constants.TRANSCRIPT_DATA['overwatch'])
         else:
             self.assertEqual(video_transcript.transcript.name, file_name)
@@ -2655,7 +2657,7 @@ def test_create_or_update_video_exceptions(self, video_id, file_format, provider
                 'file_format': file_format
             })
 
-        self.assertEqual(transcript_exception.exception.message, exception_message)
+        self.assertEqual(transcript_exception.exception.args[0], exception_message)
 
     @mock.patch.object(VideoTranscript, 'save')
     def test_create_or_update_transcript_exception_on_update(self, mock_save):
@@ -2682,7 +2684,7 @@ def test_create_or_update_transcript_exception_on_update(self, mock_save):
         # Assert that there are no updates to the transcript data
         video_transcript = VideoTranscript.objects.get(video__edx_video_id=edx_video_id,
                                                        language_code=language_code)
-        with open(video_transcript.transcript.name) as saved_transcript:
+        with open(video_transcript.transcript.name, 'rb') as saved_transcript:
             self.assertNotEqual(saved_transcript.read(), constants.TRANSCRIPT_DATA['overwatch'])
 
     @mock.patch.object(VideoTranscript, 'save')
@@ -2743,7 +2745,7 @@ def test_create_video_transcript(self):
         self.assertIsNotNone(video_transcript)
         self.assertEqual(video_transcript.file_format, transcript_props['file_format'])
         self.assertEqual(video_transcript.provider, transcript_props['provider'])
-        with open(video_transcript.transcript.name) as created_transcript:
+        with open(video_transcript.transcript.name, encoding='utf8') as created_transcript:
             self.assertEqual(created_transcript.read(), constants.TRANSCRIPT_DATA['overwatch'])
 
     @data(
@@ -2770,15 +2772,15 @@ def test_create_video_transcript_exceptions(self, video_id, language_code, file_
         with self.assertRaises(ValCannotCreateError) as transcript_exception:
             api.create_video_transcript(video_id, language_code, file_format, ContentFile(constants.TRANSCRIPT_DATA['overwatch']), provider)
 
-        self.assertIn(exception_msg, six.text_type(transcript_exception.exception.message))
+        self.assertIn(exception_msg, six.text_type(transcript_exception.exception.args[0]))
 
     def test_get_available_transcript_languages(self):
         """
         Verify that `get_available_transcript_languages` works as expected.
         """
         # `super-soaker` has got 'en' and 'fr' transcripts
         transcript_languages = api.get_available_transcript_languages(video_id=u'super-soaker')
-        self.assertItemsEqual(transcript_languages, ['en', 'fr'])
+        self.assertEqual(transcript_languages, ['en', 'fr'])
 
     @patch('edxval.api.logger')
     def test_delete_video_transcript(self, mock_logger):
@@ -2835,7 +2837,7 @@ def test_create_transcript_file(self):
         self.assertTrue(transcript_file_name in file_system.listdir(constants.EXPORT_IMPORT_STATIC_DIR))
 
         # Also verify the content of created transcript file.
-        expected_transcript_content = File(open(expected_transcript_path)).read()
+        expected_transcript_content = File(open(expected_transcript_path, 'rb')).read()
         transcript = api.get_video_transcript_data(video_id=video_id, language_code=language_code)
         self.assertEqual(transcript['content'], expected_transcript_content)
 

edxval/tests/test_serializers.py

@@ -98,8 +98,8 @@ def test_invalid_course_id(self):
         )
         self.assertFalse(serializer.is_valid())
         self.assertEqual(
-            serializer.errors,
-            {"courses": ["Ensure this value has at most 255 characters (it has 300)."]}
+            str(serializer.errors['courses'][0]),
+            "Ensure this value has at most 255 characters (it has 300)."
         )
 
     def test_encoded_video_set_output(self):

edxval/tests/test_transcript_utils.py

@@ -20,7 +20,7 @@ class TestTranscriptUtils(unittest.TestCase):
     def setUp(self):
         super(TestTranscriptUtils, self).setUp()
 
-        self.srt_transcript = textwrap.dedent("""\
+        self.srt_transcript = textwrap.dedent(u"""\
             0
             00:00:10,500 --> 00:00:13,000
             Elephant's Dream 大象的梦想
@@ -29,9 +29,9 @@ def setUp(self):
             00:00:15,000 --> 00:00:18,000
             At the left we can see...
 
-        """)
+        """).encode('utf8')
 
-        self.sjson_transcript = textwrap.dedent("""\
+        self.sjson_transcript = textwrap.dedent(u"""\
             {
                 "start": [
                     10500,
@@ -46,7 +46,7 @@ def setUp(self):
                     "At the left we can see..."
                 ]
             }
-        """)
+        """).encode('utf8')
 
     @ddt.data(
         ('invalid_input_format', 'sjson'),
@@ -90,6 +90,6 @@ def test_convert_invalid_srt_to_sjson(self):
         Tests that TranscriptsGenerationException was raises on trying
         to convert invalid srt transcript to sjson.
         """
-        invalid_srt_transcript = 'invalid SubRip file content'
+        invalid_srt_transcript = b'invalid SubRip file content'
         with self.assertRaises(TranscriptsGenerationException):
             Transcript.convert(invalid_srt_transcript, 'srt', 'sjson')

edxval/tests/test_views.py

@@ -781,7 +781,7 @@ def test_update_auto_generated_images(self):
         },
         {
             'post_data': {'course_id': 'test_course_id', 'edx_video_id': 'super-soaker', 'generated_images': [1, 2, 3]},
-            'message': "[u'list must only contain strings.']"
+            'message': "list must only contain strings.']"
         },
     )
     @unpack
@@ -793,9 +793,9 @@ def test_update_error_responses(self, post_data, message):
 
         response = self.client.post(url, post_data, format='json')
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
-        self.assertEqual(
-            response.data['message'],
-            message
+        self.assertIn(
+            message,
+            response.data['message']
         )
 
 

requirements/base.in

@@ -1,11 +1,12 @@
 # Core requirements for using this application
 
 boto
-Django>=1.8,<2
+Django>=1.11,<2
 django-model-utils
 edx-django-oauth2-provider
--e git+https://github.com/edx/django-rest-framework.git@1ceda7c086fddffd1c440cc86856441bbf0bd9cb#egg=djangorestframework==3.6.3
--e git+https://github.com/edx/django-rest-framework-oauth.git@f0b503fda8c254a38f97fef802ded4f5fe367f7a#egg=djangorestframework-oauth
+djangorestframework==3.7.7
+edx-drf-extensions
+-e git+https://github.com/edx/django-rest-framework-oauth.git@0a43e8525f1e3048efe4bc70c03de308a277197c#egg=djangorestframework-oauth==1.1.1
 django-storages
 enum34
 lxml