fix: download a swagger file from given S3 location (aws#444)

mcasado · sanathkr · commit 20c235e4efe8 · 2018-05-31T13:43:18.000-07:00
diff --git a/samcli/commands/local/lib/swagger/reader.py b/samcli/commands/local/lib/swagger/reader.py
@@ -213,7 +213,7 @@ def _download_from_s3(bucket, key, version=None):
 
         with tempfile.TemporaryFile() as fp:
             try:
-                s3.download_file(
+                s3.download_fileobj(
                     bucket, key, fp,
                     ExtraArgs=extra_args)
 
diff --git a/tests/unit/commands/local/lib/swagger/test_reader.py b/tests/unit/commands/local/lib/swagger/test_reader.py
@@ -259,12 +259,18 @@ def test_must_download_file_from_s3(self, tempfilemock, botomock):
         actual = SamSwaggerReader._download_from_s3(self.bucket, self.key, self.version)
         self.assertEquals(actual, expected)
 
-        s3_mock.download_file.assert_called_with(self.bucket, self.key, fp_mock,
-                                                 ExtraArgs={"VersionId": self.version})
+        s3_mock.download_fileobj.assert_called_with(self.bucket, self.key, fp_mock,
+                                                    ExtraArgs={"VersionId": self.version})
 
         fp_mock.seek.assert_called_with(0)  # make sure we seek the file before reading
         fp_mock.read.assert_called_with()
 
+    def test_must_fail_on_download_from_s3(self):
+        with self.assertRaises(Exception) as cm:
+            SamSwaggerReader._download_from_s3(self.bucket, self.key)
+        self.assertIn(cm.exception.__class__,
+                      (botocore.exceptions.NoCredentialsError, botocore.exceptions.ClientError))
+
     @patch('samcli.commands.local.lib.swagger.reader.boto3')
     @patch('samcli.commands.local.lib.swagger.reader.tempfile')
     def test_must_work_without_object_version_id(self, tempfilemock, botomock):
@@ -281,8 +287,8 @@ def test_must_work_without_object_version_id(self, tempfilemock, botomock):
         actual = SamSwaggerReader._download_from_s3(self.bucket, self.key)
         self.assertEquals(actual, expected)
 
-        s3_mock.download_file.assert_called_with(self.bucket, self.key, fp_mock,
-                                                 ExtraArgs={})
+        s3_mock.download_fileobj.assert_called_with(self.bucket, self.key, fp_mock,
+                                                    ExtraArgs={})
 
     @patch('samcli.commands.local.lib.swagger.reader.boto3')
     @patch('samcli.commands.local.lib.swagger.reader.tempfile')
@@ -293,8 +299,8 @@ def test_must_log_on_download_exception(self, tempfilemock, botomock):
 
         fp_mock = Mock()
         tempfilemock.TemporaryFile.return_value.__enter__.return_value = fp_mock  # mocking context manager
-        s3_mock.download_file.side_effect = botocore.exceptions.ClientError({"Error": {}},
-                                                                            "download_file")
+        s3_mock.download_fileobj.side_effect = botocore.exceptions.ClientError({"Error": {}},
+                                                                               "download_file")
 
         with self.assertRaises(botocore.exceptions.ClientError):
             SamSwaggerReader._download_from_s3(self.bucket, self.key)
