Merge pull request #146 from mpacer/read_content_in_client

Move async content reading inside s3 client context manager

Author: rgbkrk

bookstore/clone.py

@@ -197,13 +197,14 @@ async def _clone(self, s3_bucket, s3_object_key):
             self.log.info(f"Processing clone of {s3_object_key}")
             try:
                 obj = await client.get_object(Bucket=s3_bucket, Key=s3_object_key)
+                content = (await obj['Body'].read()).decode('utf-8')
             except ClientError as e:
                 status_code = e.response['ResponseMetadata'].get('HTTPStatusCode')
                 raise web.HTTPError(status_code, e.args[0])
 
             self.log.info(f"Obtained contents for {s3_object_key}")
 
-        return obj
+        return obj, content
 
     @web.authenticated
     async def post(self):
@@ -237,9 +238,9 @@ async def post(self):
         )
 
         self.log.info(f"About to clone from {s3_object_key}")
-        obj = await self._clone(s3_bucket, s3_object_key)
+        obj, content = await self._clone(s3_bucket, s3_object_key)
 
-        content_model = await self.build_content_model(obj, target_path)
+        content_model = self.build_content_model(content, target_path)
 
         self.log.info(f"Completing clone for {s3_object_key}")
         self.contents_manager.save(content_model, content_model['path'])
@@ -250,15 +251,15 @@ async def post(self):
         self.set_header('Content-Type', 'application/json')
         self.finish(resp_model)
 
-    async def build_content_model(self, obj, target_path):
+    def build_content_model(self, content, target_path):
         """Helper that takes a response from S3 and creates a ContentsAPI compatible model.
         
         If the file at target_path already exists, this increments the file name.
         
         Parameters
         ----------
-        obj : dict
-            Response object from S3
+        content : str
+            string encoded file content
         target_path : str
             The the path we wish to clone to, may be incremented if already present.
 
@@ -268,8 +269,6 @@ async def build_content_model(self, obj, target_path):
             Jupyter Contents API compatible model
         """
         path = self.contents_manager.increment_filename(target_path)
-        content = await obj['Body'].read()
-        content = content.decode('utf-8')
         if os.path.splitext(path)[1] in [".ipynb", ".jpynb"]:
             model = build_notebook_model(content, path)
         else:

bookstore/tests/test_clone.py

@@ -235,17 +235,9 @@ async def test_build_text_content_model(self):
             "path": "test_directory/file_name.txt",
         }
 
-        class MyFakeClass:
-            def __init__(self):
-                pass
-
-            async def read(self):
-                return content.encode('utf-8')
-
-        obj = {'Body': MyFakeClass()}
         path = "test_directory/file_name.txt"
         success_handler = self.post_handler({})
-        model = await success_handler.build_content_model(obj, path)
+        model = success_handler.build_content_model(content, path)
         assert model == expected
 
     @gen_test
@@ -259,15 +251,9 @@ async def test_build_notebook_content_model(self):
             "path": "test_directory/file_name.ipynb",
         }
 
-        class MyFakeClass:
-            def __init__(self):
-                pass
-
-            async def read(self):
-                return nbformat.writes(content).encode('utf-8')
+        str_content = nbformat.writes(content)
 
-        obj = {'Body': MyFakeClass()}
         path = "test_directory/file_name.ipynb"
         success_handler = self.post_handler({})
-        model = await success_handler.build_content_model(obj, path)
+        model = success_handler.build_content_model(str_content, path)
         assert model == expected