Merge pull request #1037 from edx/efischer/back_to_boto

Revert boto3 upgrade

openassessment/fileupload/backends/s3.py

 import logging
 
-import boto3
+import boto
 
 from django.conf import settings
 
@@ -15,16 +15,15 @@ class Backend(BaseBackend):
     def get_upload_url(self, key, content_type):
         bucket_name, key_name = self._retrieve_parameters(key)
         try:
-            client = _connect_to_s3()
-            return client.generate_presigned_url(
-                ExpiresIn=self.UPLOAD_URL_TIMEOUT,
-                ClientMethod='put_object',
-                Params={
-                    'Bucket': bucket_name,
-                    'Key': key_name
-                },
-                HttpMethod="PUT"
+            conn = _connect_to_s3()
+            upload_url = conn.generate_url(
+                expires_in=self.UPLOAD_URL_TIMEOUT,
+                method='PUT',
+                bucket=bucket_name,
+                key=key_name,
+                headers={'Content-Length': '5242880', 'Content-Type': content_type}
             )
+            return upload_url
         except Exception as ex:
             logger.exception(
                 u"An internal exception occurred while generating an upload URL."
@@ -34,16 +33,10 @@ class Backend(BaseBackend):
     def get_download_url(self, key):
         bucket_name, key_name = self._retrieve_parameters(key)
         try:
-            client = _connect_to_s3()
-            return client.generate_presigned_url(
-                ExpiresIn=self.DOWNLOAD_URL_TIMEOUT,
-                ClientMethod='get_object',
-                Params={
-                    'Bucket': bucket_name,
-                    'Key': key_name
-                },
-                HttpMethod="GET"
-            )
+            conn = _connect_to_s3()
+            bucket = conn.get_bucket(bucket_name)
+            s3_key = bucket.get_key(key_name)
+            return s3_key.generate_url(expires_in=self.DOWNLOAD_URL_TIMEOUT) if s3_key else ""
         except Exception as ex:
             logger.exception(
                 u"An internal exception occurred while generating a download URL."
@@ -52,16 +45,14 @@ class Backend(BaseBackend):
 
     def remove_file(self, key):
         bucket_name, key_name = self._retrieve_parameters(key)
-        client = _connect_to_s3()
-        resp = client.delete_objects(
-            Bucket=bucket_name,
-            Delete={
-                'Objects': [{'Key':key_name}]
-            }
-        )
-        if 'Deleted' in resp and any(key_name == deleted_dict['Key'] for deleted_dict in resp['Deleted']):
+        conn = _connect_to_s3()
+        bucket = conn.get_bucket(bucket_name)
+        s3_key = bucket.get_key(key_name)
+        if s3_key:
+            bucket.delete_key(s3_key)
             return True
-        return False
+        else:
+            return False
 
 
 def _connect_to_s3():
@@ -76,8 +67,7 @@ def _connect_to_s3():
     aws_access_key_id = getattr(settings, 'AWS_ACCESS_KEY_ID', None)
     aws_secret_access_key = getattr(settings, 'AWS_SECRET_ACCESS_KEY', None)
 
-    return boto3.client(
-        's3',
+    return boto.connect_s3(
         aws_access_key_id=aws_access_key_id,
         aws_secret_access_key=aws_secret_access_key
     )

openassessment/fileupload/tests/test_api.py

@@ -7,7 +7,8 @@ import tempfile
 import urllib
 from urlparse import urlparse
 
-import boto3
+import boto
+from boto.s3.key import Key
 import ddt
 from mock import Mock, patch
 from moto import mock_s3
@@ -35,8 +36,8 @@ class TestFileUploadService(TestCase):
         FILE_UPLOAD_STORAGE_BUCKET_NAME="mybucket"
     )
     def test_get_upload_url(self):
-        s3 = boto3.resource('s3')
-        s3.create_bucket(Bucket='mybucket')
+        conn = boto.connect_s3()
+        conn.create_bucket('mybucket')
         uploadUrl = api.get_upload_url("foo", "bar")
         self.assertIn("https://mybucket.s3.amazonaws.com/submissions_attachments/foo", uploadUrl)
 
@@ -47,9 +48,11 @@ class TestFileUploadService(TestCase):
         FILE_UPLOAD_STORAGE_BUCKET_NAME="mybucket"
     )
     def test_get_download_url(self):
-        s3 = boto3.resource('s3')
-        s3.create_bucket(Bucket='mybucket')
-        s3.Object('mybucket', 'submissions_attachments/foo').put(Body="How d'ya do?")
+        conn = boto.connect_s3()
+        bucket = conn.create_bucket('mybucket')
+        key = Key(bucket)
+        key.key = "submissions_attachments/foo"
+        key.set_contents_from_string("How d'ya do?")
         downloadUrl = api.get_download_url("foo")
         self.assertIn("https://mybucket.s3.amazonaws.com/submissions_attachments/foo", downloadUrl)
 
@@ -60,9 +63,11 @@ class TestFileUploadService(TestCase):
         FILE_UPLOAD_STORAGE_BUCKET_NAME="mybucket"
     )
     def test_remove_file(self):
-        s3 = boto3.resource('s3')
-        s3.create_bucket(Bucket='mybucket')
-        s3.Object('mybucket', 'submissions_attachments/foo').put(Body="Test")
+        conn = boto.connect_s3()
+        bucket = conn.create_bucket('mybucket')
+        key = Key(bucket)
+        key.key = "submissions_attachments/foo"
+        key.set_contents_from_string("Test")
         result = api.remove_file("foo")
         self.assertTrue(result)
         result = api.remove_file("foo")
@@ -82,7 +87,7 @@ class TestFileUploadService(TestCase):
         AWS_SECRET_ACCESS_KEY='bizbaz',
         FILE_UPLOAD_STORAGE_BUCKET_NAME="mybucket"
     )
-    @patch.object(boto3, 'client')
+    @patch.object(boto, 'connect_s3')
     @raises(exceptions.FileUploadInternalError)
     def test_get_upload_url_error(self, mock_s3):
         mock_s3.side_effect = Exception("Oh noes")
@@ -94,7 +99,7 @@ class TestFileUploadService(TestCase):
         AWS_SECRET_ACCESS_KEY='bizbaz',
         FILE_UPLOAD_STORAGE_BUCKET_NAME="mybucket"
     )
-    @patch.object(boto3, 'client')
+    @patch.object(boto, 'connect_s3')
     @raises(exceptions.FileUploadInternalError, mock_s3)
     def test_get_download_url_error(self, mock_s3):
         mock_s3.side_effect = Exception("Oh noes")

openassessment/management/commands/upload_oa_data.py

@@ -9,7 +9,8 @@ import sys
 import tarfile
 import tempfile
 
-import boto3
+import boto
+from boto.s3.key import Key
 
 from django.conf import settings
 from django.core.management.base import BaseCommand, CommandError
@@ -137,24 +138,16 @@ class Command(BaseCommand):
         # environment vars or configuration files instead.
         aws_access_key_id = getattr(settings, 'AWS_ACCESS_KEY_ID', None)
         aws_secret_access_key = getattr(settings, 'AWS_SECRET_ACCESS_KEY', None)
-        client = boto3.client(
-            's3',
+        conn = boto.connect_s3(
             aws_access_key_id=aws_access_key_id,
             aws_secret_access_key=aws_secret_access_key
         )
-        bucket = client.create_bucket(Bucket=s3_bucket)
 
+        bucket = conn.get_bucket(s3_bucket)
         key_name = os.path.join(course_id, os.path.split(file_path)[1])
-        client.put_object(Bucket=s3_bucket, Key=key_name, Body=open(file_path, 'rb'))
-        url = client.generate_presigned_url(
-            ExpiresIn=self.URL_EXPIRATION_HOURS * 3600,
-            ClientMethod='get_object',
-            Params={
-                'Bucket': s3_bucket,
-                'Key': key_name
-            },
-            HttpMethod="GET"
-        )
+        key = Key(bucket=bucket, name=key_name)
+        key.set_contents_from_filename(file_path)
+        url = key.generate_url(self.URL_EXPIRATION_HOURS * 3600)
 
         # Store the key and url in the history
         self._history.append({'key': key_name, 'url': url})

openassessment/management/tests/test_upload_oa_data.py

@@ -2,9 +2,10 @@
 """
 Tests for management command that uploads submission/assessment data.
 """
+from StringIO import StringIO
 import tarfile
 
-import boto3
+import boto
 import moto
 
 from openassessment.management.commands import upload_oa_data
@@ -30,8 +31,8 @@ class UploadDataTest(CacheResetTest):
     @moto.mock_s3
     def test_upload(self):
         # Create an S3 bucket using the fake S3 implementation
-        s3 = boto3.resource('s3')
-        s3.create_bucket(Bucket=self.BUCKET_NAME)
+        conn = boto.connect_s3()
+        conn.create_bucket(self.BUCKET_NAME)
 
         # Create some submissions to ensure that we cover
         # the progress indicator code.
@@ -54,10 +55,12 @@ class UploadDataTest(CacheResetTest):
 
         # Retrieve the uploaded file from the fake S3 implementation
         self.assertEqual(len(cmd.history), 1)
-        s3.Object(self.BUCKET_NAME, cmd.history[0]['key']).download_file("tmp-test-file.tar.gz")
+        bucket = conn.get_all_buckets()[0]
+        key = bucket.get_key(cmd.history[0]['key'])
+        contents = StringIO(key.get_contents_as_string())
 
         # Expect that the contents contain all the expected CSV files
-        with tarfile.open("tmp-test-file.tar.gz", mode="r:gz") as tar:
+        with tarfile.open(mode="r:gz", fileobj=contents) as tar:
             file_sizes = {
                 member.name: member.size
                 for member in tar.getmembers()
@@ -68,4 +71,4 @@ class UploadDataTest(CacheResetTest):
 
         # Expect that we generated a URL for the bucket
         url = cmd.history[0]['url']
-        self.assertIn("https://s3.amazonaws.com/{}".format(self.BUCKET_NAME), url)
+        self.assertIn("https://{}".format(self.BUCKET_NAME), url)

openassessment/xblock/test/test_leaderboard.py

@@ -6,7 +6,8 @@ import json
 from random import randint
 from urlparse import urlparse
 
-import boto3
+import boto
+from boto.s3.key import Key
 import mock
 from moto import mock_s3
 
@@ -146,15 +147,15 @@ class TestLeaderboardRender(XBlockHandlerTransactionTestCase):
     @scenario('data/leaderboard_show.xml')
     def test_non_text_submission(self, xblock):
         # Create a mock bucket
-        s3 = boto3.resource('s3')
-        s3.create_bucket(Bucket='mybucket')
+        conn = boto.connect_s3()
+        bucket = conn.create_bucket('mybucket')
         # Create a non-text submission (the submission dict doesn't contain 'text')
         file_download_url = api.get_download_url('s3key')
         self._create_submissions_and_scores(xblock, [('s3key', 1)], submission_key='file_key')
 
         # Expect that we default to an empty string for content
         self._assert_scores(xblock, [
-            {'score': 1, 'files': [(file_download_url, '')], 'submission': ''}
+            {'score': 1, 'files': [], 'submission': ''}
         ])
 
     @mock_s3
@@ -171,10 +172,11 @@ class TestLeaderboardRender(XBlockHandlerTransactionTestCase):
         file_keys = ['foo', 'bar']
         file_descriptions = ['{}-description'.format(file_key) for file_key in file_keys]
 
-        s3 = boto3.resource('s3')
-        s3.create_bucket(Bucket='mybucket')
+        conn = boto.connect_s3()
+        bucket = conn.create_bucket('mybucket')
         for file_key in file_keys:
-            s3.Object('mybucket', 'submissions_attachments/{}'.format(file_key)).put(Body="How d'ya do?")
+            key = Key(bucket, 'submissions_attachments/{}'.format(file_key))
+            key.set_contents_from_string("How d'ya do?")
             files_url_and_description = [
                 (api.get_download_url(file_key), file_descriptions[idx])
                 for idx, file_key in enumerate(file_keys)
@@ -210,9 +212,10 @@ class TestLeaderboardRender(XBlockHandlerTransactionTestCase):
         Tests that text and image submission works as expected
         """
         # Create a file and get the download URL
-        s3 = boto3.resource('s3')
-        s3.create_bucket(Bucket='mybucket')
-        s3.Object('mybucket', 'submissions_attachments/foo').put(Body="How d'ya do?")
+        conn = boto.connect_s3()
+        bucket = conn.create_bucket('mybucket')
+        key = Key(bucket, 'submissions_attachments/foo')
+        key.set_contents_from_string("How d'ya do?")
 
         file_download_url = [(api.get_download_url('foo'), '')]
         # Create a image and text submission

openassessment/xblock/test/test_submission.py

@@ -6,7 +6,8 @@ Test submission to the OpenAssessment XBlock.
 import datetime as dt
 import json
 
-import boto3
+import boto
+from boto.s3.key import Key
 from mock import Mock, patch
 from moto import mock_s3
 import pytz
@@ -157,14 +158,11 @@ class SubmissionTest(XBlockHandlerTestCase):
     @scenario('data/file_upload_scenario.xml')
     def test_download_url(self, xblock):
         """ Test generate correct download URL with existing file. should create a file and get the download URL """
-        s3 = boto3.resource('s3')
-        s3.create_bucket(Bucket='mybucket')
-        s3.Object(
-            'mybucket',
-            'submissions_attachments/test_student/test_course/' + xblock.scope_ids.usage_id
-        ).put(
-            Body="How d'ya do?"
-        )
+        conn = boto.connect_s3()
+        bucket = conn.create_bucket('mybucket')
+        key = Key(bucket)
+        key.key = "submissions_attachments/test_student/test_course/" + xblock.scope_ids.usage_id
+        key.set_contents_from_string("How d'ya do?")
         download_url = api.get_download_url("test_student/test_course/" + xblock.scope_ids.usage_id)
 
         xblock.xmodule_runtime = Mock(
@@ -189,7 +187,7 @@ class SubmissionTest(XBlockHandlerTestCase):
         resp = self.request(xblock, 'download_url', json.dumps(dict()), response_format='json')
 
         self.assertTrue(resp['success'])
-        self.assertIn(u'https://mybucket.s3.amazonaws.com/submissions_attachments', resp['url'])
+        self.assertEqual(u'', resp['url'])
 
     @mock_s3
     @override_settings(
@@ -200,11 +198,11 @@ class SubmissionTest(XBlockHandlerTestCase):
     @scenario('data/file_upload_scenario.xml')
     def test_remove_all_uploaded_files(self, xblock):
         """ Test remove all user files """
-        s3 = boto3.resource('s3')
-        s3.create_bucket(Bucket='mybucket')
-        s3.Object('mybucket', 'submissions_attachments/test_student/test_course/' + xblock.scope_ids.usage_id).put(
-            Body="How d'ya do?"
-        )
+        conn = boto.connect_s3()
+        bucket = conn.create_bucket('mybucket')
+        key = Key(bucket)
+        key.key = "submissions_attachments/test_student/test_course/" + xblock.scope_ids.usage_id
+        key.set_contents_from_string("How d'ya do?")
 
         xblock.xmodule_runtime = Mock(
             course_id='test_course',
@@ -221,7 +219,7 @@ class SubmissionTest(XBlockHandlerTestCase):
 
         resp = self.request(xblock, 'download_url', json.dumps(dict()), response_format='json')
         self.assertTrue(resp['success'])
-        self.assertIn(u'https://mybucket.s3.amazonaws.com/submissions_attachments', resp['url'])
+        self.assertEqual(u'', resp['url'])
 
     @mock_s3
     @override_settings(

requirements/base.txt

@@ -5,7 +5,7 @@ git+https://github.com/edx/XBlock.git@xblock-1.0.1#egg=XBlock==1.0.1
 git+https://github.com/edx/xblock-sdk.git@v0.1.4#egg=xblock-sdk==0.1.4
 
 # Third Party Requirements
-boto3>=1.4.4,<2.0.0
+boto>=2.48.0,<3.0.0
 python-swiftclient>=3.1.0,<4.0.0
 defusedxml>=0.4.1,<1.0.0
 django-model-utils>=2.3.1

requirements/test.txt

@@ -5,7 +5,7 @@ ddt==1.0.0
 factory_boy==2.8.1
 freezegun==0.3.9
 mock==2.0.0
-moto==1.0.1
+moto==0.4.31
 nose==1.3.7
 tox==2.7.0
 

test/acceptance/pages.py

@@ -93,7 +93,7 @@ class OpenAssessmentPage(BaseAssessmentPage):
         """
         submit_button_selector = self._bounded_selector(button_css)
         EmptyPromise(
-            lambda: False == any(self.q(css=submit_button_selector).attrs('disabled')),
+            lambda: not any(self.q(css=submit_button_selector).attrs('disabled')),
             "Submit button is enabled."
         ).fulfill()
 
@@ -286,7 +286,7 @@ class AssessmentMixin(object):
         attempts = 0
         while not criterion_selected() and attempts < 5:
             select_criterion()
-            attempts+=1
+            attempts += 1
 
         self.submit_assessment()
         return self
@@ -695,14 +695,20 @@ class StaffAreaPage(OpenAssessmentPage, AssessmentMixin):
         self.q(css=student_input_css).fill(username)
         submit_button = self.q(css=self._bounded_selector(".action--submit-username"))
         submit_button.first.click()
-        self.wait_for_element_visibility(self._bounded_selector(".staff-info__student__report"), "Student report is present")
+        self.wait_for_element_visibility(
+            self._bounded_selector(".staff-info__student__report"),
+            "Student report is present"
+        )
 
     def expand_staff_grading_section(self):
         """
         Clicks the staff grade control to expand staff grading section for use in staff required workflows.
         """
         self.q(css=self._bounded_selector(".staff__grade__show-form")).first.click()
-        self.wait_for_element_visibility(".staff-full-grade__assessment__rubric__question--0", "staff grading is present")
+        self.wait_for_element_visibility(
+            ".staff-full-grade__assessment__rubric__question--0",
+            "staff grading is present"
+        )
 
     @property
     def available_checked_out_numbers(self):
@@ -718,7 +724,8 @@ class StaffAreaPage(OpenAssessmentPage, AssessmentMixin):
 
     def verify_available_checked_out_numbers(self, expected_value):
         """
-        Waits until the expected value for available and checked out numbers appears. If it does not appear, fails the test.
+        Waits until the expected value for available and checked out numbers appears. If it does not appear, fails the
+        test.
 
         expected_value should be a tuple as described in the available_checked_out_numbers property above.
         """
@@ -863,7 +870,9 @@ class StaffAreaPage(OpenAssessmentPage, AssessmentMixin):
 
         Returns: the text present in "Overall Feedback"
         """
-        return self.q(css=self._bounded_selector(".staff-info__{} .student__answer__display__content".format(section))).text[0]
+        return self.q(
+            css=self._bounded_selector(".staff-info__{} .student__answer__display__content".format(section))
+        ).text[0]
 
     def _get_table_text(self, selector):
         """

test/acceptance/tests.py

@@ -20,6 +20,7 @@ from acceptance.pages import AssessmentPage, GradePage, StaffAreaPage, Submissio
 # This value is generally used in jenkins, but not locally
 PROFILING_ENABLED = os.environ.get('ORA_PROFILING_ENABLED', False)
 
+
 def retry(tries=2, delay=4, backoff=2):
     """
     Retry decorator with exponential backoff.