From ff19351e275a2b2018629e9c17b93b7ff2dc42ea Mon Sep 17 00:00:00 2001 From: Amanda Kaufman Date: Wed, 14 Nov 2018 11:26:24 -0700 Subject: [PATCH 1/9] Added exception handling for copy/delete blob operations. --- functions/pipeline/onboarding/__init__.py | 52 +++++++++---------- .../pipeline/onboarding/onboarding-client.py | 39 -------------- 2 files changed, 26 insertions(+), 65 deletions(-) delete mode 100644 functions/pipeline/onboarding/onboarding-client.py diff --git a/functions/pipeline/onboarding/__init__.py b/functions/pipeline/onboarding/__init__.py index 7e87ba9a..a8dc50c9 100644 --- a/functions/pipeline/onboarding/__init__.py +++ b/functions/pipeline/onboarding/__init__.py @@ -26,8 +26,6 @@ def main(req: func.HttpRequest) -> func.HttpResponse: print("Unable to decode JSON body") return func.HttpResponse("Unable to decode POST body", status_code=400) - logging.error(req_body) - # Build list of image objects to pass to DAL for insertion into DB. image_object_list = [] image_name_list = [] @@ -84,39 +82,40 @@ def main(req: func.HttpRequest) -> func.HttpResponse: new_blob_name = (str(image_id) + file_extension) copy_from_container = os.getenv('SOURCE_CONTAINER_NAME') copy_to_container = os.getenv('DESTINATION_CONTAINER_NAME') - permanent_storage_path = "https://{0}.blob.core.windows.net/{0}/{1}".format(copy_from_container, new_blob_name) # Verbose logging for testing - logging.info("Original image URL: " + original_image_url) logging.info("Original image name: " + original_blob_name) - logging.info("File extension: " + file_extension) logging.info("Image ID: " + str(image_id)) logging.info("New blob name: " + new_blob_name) - logging.info("Now copying file from temporary to permanent storage...") - logging.info("Permanent image URL: " + permanent_storage_path) + # Create the blob URLs blob_service = BlockBlobService(account_name=os.getenv('STORAGE_ACCOUNT_NAME'), account_key=os.getenv('STORAGE_ACCOUNT_KEY')) - source_blob_url = blob_service.make_blob_url(copy_from_container, original_blob_name) - - # TODO: Exception handling in case blob cannot be copied for some reason. - blob_service.copy_blob(copy_to_container, new_blob_name, source_blob_url) - logging.info("Done.") + source_blob_path = blob_service.make_blob_url(copy_from_container, original_blob_name) + destination_blob_path = blob_service.make_blob_url(copy_to_container, new_blob_name) - # Delete the file from temp storage once it's been copied - # Note: The following delete code works. Commenting out for testing of other functions. - ''' - logging.info("Now deleting image " + original_blob_name + " from temp storage container.") + # Copy blob from temp storage to permanent storage try: - blob_service.delete_blob(copy_from_container, original_blob_name) - print("Blob " + original_blob_name + " has been deleted successfully") - except: - print("Blob " + original_blob_name + " deletion failed") - ''' - - # Add image to the list of images to be returned in the response - permanent_url_list.append(permanent_storage_path) - # Add ImageId and permanent storage url to new dictionary to be sent to update function - update_urls_dictionary[image_id] = permanent_storage_path + logging.info("Now copying file from temporary to permanent storage...") + logging.info("Source path: " + source_blob_path) + logging.info("Destination path: " + destination_blob_path) + blob_service.copy_blob(copy_to_container, new_blob_name, source_blob_path) + logging.info("Done.") + + # Add image to the list of images to be returned in the response + permanent_url_list.append(destination_blob_path) + # Add ImageId and permanent storage url to new dictionary to be sent to update function + update_urls_dictionary[image_id] = destination_blob_path + + # Delete the file from temp storage once it's been copied + logging.info("Now deleting image " + original_blob_name + " from temp storage container.") + try: + blob_service.delete_blob(copy_from_container, original_blob_name) + logging.info("Blob " + original_blob_name + " has been deleted successfully") + except Exception as e: + logging.error("ERROR: Deletion of blob " + original_blob_name + " failed. Exception: " + str(e)) + + except Exception as e: + logging.error("ERROR: Copy of blob " + original_blob_name + " failed. Exception: " + str(e)) logging.info("Now updating permanent URLs in the DB...") data_access.update_image_urls(update_urls_dictionary, user_id_number) @@ -128,3 +127,4 @@ def main(req: func.HttpRequest) -> func.HttpResponse: # Return string containing list of URLs to images in permanent blob storage return func.HttpResponse("The following images should now be added to the DB and exist in permanent blob storage: \n" + permanent_url_string, status_code=200) + diff --git a/functions/pipeline/onboarding/onboarding-client.py b/functions/pipeline/onboarding/onboarding-client.py deleted file mode 100644 index bb7243a8..00000000 --- a/functions/pipeline/onboarding/onboarding-client.py +++ /dev/null @@ -1,39 +0,0 @@ -import requests -import json -import pg8000 - -# The following mock client imitates the CLI during the onboarding scenario for new images. -# The expectation is that the CLI uploads images to a temporary blob store, then gets a list -# of URLs to those images and passes the list to an HTTP trigger function in the format of -# a JSON string. The HTTP trigger function creates rows in the database for the images, -# retrieves the ImageId's for them, and then copies the images, each renamed as "ImageId.extension", -# into a permanent blob storage container. The HTTP function returns the list of URLs to -# the images in permanent blob storage. - -print("\nTest client for CLI Onboarding scenario") -print('-' * 40) - -# functionURL = "http://localhost:7071/api/onboarding?userId=aka" -functionURL = "https://onboardinghttptrigger.azurewebsites.net/api/onboarding?userId=aka" - -urlList = { "imageUrls": ["https://akaonboardingstorage.blob.core.windows.net/aka-temp-source-container/puppies1.jpg", - "https://akaonboardingstorage.blob.core.windows.net/aka-temp-source-container/puppies2.jpg", - "https://akaonboardingstorage.blob.core.windows.net/aka-temp-source-container/puppies3.jpg", - "https://akaonboardingstorage.blob.core.windows.net/aka-temp-source-container/puppies2.jpg"] } - -headers = {"Content-Type": "application/json"} - -print("Now executing POST request to onboard images...to:") -print("Function URL: " + functionURL) -print("Headers:") -for key, value in headers.items(): - print("\t" + key + ": " + value) -response = requests.post(url=functionURL, headers=headers, json=urlList) -print("Completed POST request.") - -raw_response = response.text -response_array = raw_response.split(", ") -response_output = "\n".join(response_array) - -print(f"Response status code: {response.status_code}") -print(f"Response string: {response_output}") From 75146e0f204b90b2a37b24e0f17b0a4294172a48 Mon Sep 17 00:00:00 2001 From: Amanda Kaufman Date: Wed, 14 Nov 2018 11:39:35 -0700 Subject: [PATCH 2/9] Added exception handling for connection database. --- functions/pipeline/onboarding/__init__.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/functions/pipeline/onboarding/__init__.py b/functions/pipeline/onboarding/__init__.py index a8dc50c9..5e911303 100644 --- a/functions/pipeline/onboarding/__init__.py +++ b/functions/pipeline/onboarding/__init__.py @@ -6,10 +6,6 @@ from ..shared.db_access import ImageTagDataAccess, ImageInfo from azure.storage.blob import BlockBlobService -# TODO: User id as param to function - holding off until further discussion -# regarding whether user ID should be generated/looked up by the CLI or -# from within this function - def main(req: func.HttpRequest) -> func.HttpResponse: logging.info('Python HTTP trigger function processed a request.') @@ -47,9 +43,13 @@ def main(req: func.HttpRequest) -> func.HttpResponse: image_object_list.append(image) # TODO: Wrap db access section in try/catch, send an appropriate http response in the event of an error - logging.info("Now connecting to database...") - data_access = ImageTagDataAccess(get_postgres_provider()) - logging.info("Connected.") + try: + logging.info("Now connecting to database...") + data_access = ImageTagDataAccess(get_postgres_provider()) + logging.info("Connected.") + except Exception as e: + logging.error("ERROR: Database connection failed. Exception: " + str(e)) + return func.HttpResponse("Unable to connect to database", status_code=503) # Create user id user_id_number = data_access.create_user(user_id) From 4a787997c2c50de4b6ecd11a0948acd1879e9cfd Mon Sep 17 00:00:00 2001 From: Amanda Kaufman Date: Wed, 14 Nov 2018 13:19:22 -0700 Subject: [PATCH 3/9] Separated out blob manipulation code into a module --- functions/pipeline/onboarding/__init__.py | 69 +++---------------- .../shared/onboarding_module/__init__.py | 58 ++++++++++++++++ 2 files changed, 68 insertions(+), 59 deletions(-) create mode 100644 functions/pipeline/shared/onboarding_module/__init__.py diff --git a/functions/pipeline/onboarding/__init__.py b/functions/pipeline/onboarding/__init__.py index 5e911303..2d2c6ad9 100644 --- a/functions/pipeline/onboarding/__init__.py +++ b/functions/pipeline/onboarding/__init__.py @@ -4,7 +4,7 @@ from ..shared.db_provider import get_postgres_provider from ..shared.db_access import ImageTagDataAccess, ImageInfo -from azure.storage.blob import BlockBlobService +from ..shared.onboarding_module import blob_storage_manipulation def main(req: func.HttpRequest) -> func.HttpResponse: logging.info('Python HTTP trigger function processed a request.') @@ -62,69 +62,20 @@ def main(req: func.HttpRequest) -> func.HttpResponse: logging.info("Image ID and URL map dictionary:") logging.info(image_id_url_map) - # Copy over images to permanent blob store and save URLs in a list - permanent_url_list = [] - update_urls_dictionary = {} - - # TODO: Add check to make sure image exists in temp storage before attempting these operations - # TODO: Put blob storage manipulation into a separate function and add to shared codebase - # TODO: Try/catch to distinguish among errors - for key, value in image_id_url_map.items(): - - # Verbose logging for testing - logging.info("Key: " + key) - logging.info("Value: " + str(value)) - - original_image_url = key - original_blob_name = original_image_url.split("/")[-1] - file_extension = os.path.splitext(original_image_url)[1] - image_id = value - new_blob_name = (str(image_id) + file_extension) - copy_from_container = os.getenv('SOURCE_CONTAINER_NAME') - copy_to_container = os.getenv('DESTINATION_CONTAINER_NAME') - - # Verbose logging for testing - logging.info("Original image name: " + original_blob_name) - logging.info("Image ID: " + str(image_id)) - logging.info("New blob name: " + new_blob_name) - - # Create the blob URLs - blob_service = BlockBlobService(account_name=os.getenv('STORAGE_ACCOUNT_NAME'), account_key=os.getenv('STORAGE_ACCOUNT_KEY')) - source_blob_path = blob_service.make_blob_url(copy_from_container, original_blob_name) - destination_blob_path = blob_service.make_blob_url(copy_to_container, new_blob_name) - - # Copy blob from temp storage to permanent storage - try: - logging.info("Now copying file from temporary to permanent storage...") - logging.info("Source path: " + source_blob_path) - logging.info("Destination path: " + destination_blob_path) - blob_service.copy_blob(copy_to_container, new_blob_name, source_blob_path) - logging.info("Done.") - - # Add image to the list of images to be returned in the response - permanent_url_list.append(destination_blob_path) - # Add ImageId and permanent storage url to new dictionary to be sent to update function - update_urls_dictionary[image_id] = destination_blob_path - - # Delete the file from temp storage once it's been copied - logging.info("Now deleting image " + original_blob_name + " from temp storage container.") - try: - blob_service.delete_blob(copy_from_container, original_blob_name) - logging.info("Blob " + original_blob_name + " has been deleted successfully") - except Exception as e: - logging.error("ERROR: Deletion of blob " + original_blob_name + " failed. Exception: " + str(e)) - - except Exception as e: - logging.error("ERROR: Copy of blob " + original_blob_name + " failed. Exception: " + str(e)) + copy_from_container = os.getenv('SOURCE_CONTAINER_NAME') + copy_to_container = os.getenv('DESTINATION_CONTAINER_NAME') + + + update_urls_dictionary = blob_storage_manipulation(image_id_url_map, copy_from_container, copy_to_container) + logging.info("Now updating permanent URLs in the DB...") data_access.update_image_urls(update_urls_dictionary, user_id_number) logging.info("Done.") - # Construct response string of permanent URLs - permanent_url_string = (", ".join(permanent_url_list)) + # TODO: Return a JSON blob as a string containing list of successes and list of failures. If list of failures + # contains any items, return a status code other than 200. # Return string containing list of URLs to images in permanent blob storage - return func.HttpResponse("The following images should now be added to the DB and exist in permanent blob storage: \n" - + permanent_url_string, status_code=200) + return func.HttpResponse("Images were successfully added to the database and copied to permanent storage.", status_code=200) diff --git a/functions/pipeline/shared/onboarding_module/__init__.py b/functions/pipeline/shared/onboarding_module/__init__.py new file mode 100644 index 00000000..70c4a165 --- /dev/null +++ b/functions/pipeline/shared/onboarding_module/__init__.py @@ -0,0 +1,58 @@ +import os +import logging +from azure.storage.blob import BlockBlobService + +def blob_storage_manipulation(image_id_url_map, copy_from_container, copy_to_container): + # Copy over images to permanent blob store and save URLs in a list + update_urls_dictionary = {} + + # Create blob service for storage account + blob_service = BlockBlobService(account_name=os.getenv('STORAGE_ACCOUNT_NAME'), account_key=os.getenv('STORAGE_ACCOUNT_KEY')) + + # TODO: Add check to make sure image exists in temp storage before attempting these operations + # TODO: Put blob storage manipulation into a separate function and add to shared codebase + # TODO: Try/catch to distinguish among errors + for key, value in image_id_url_map.items(): + + # Verbose logging for testing + logging.info("Key: " + key) + logging.info("Value: " + str(value)) + + original_image_url = key + original_blob_name = original_image_url.split("/")[-1] + file_extension = os.path.splitext(original_image_url)[1] + image_id = value + new_blob_name = (str(image_id) + file_extension) + + # Verbose logging for testing + logging.info("Original image name: " + original_blob_name) + logging.info("Image ID: " + str(image_id)) + logging.info("New blob name: " + new_blob_name) + + # Create the blob URLs + source_blob_path = blob_service.make_blob_url(copy_from_container, original_blob_name) + destination_blob_path = blob_service.make_blob_url(copy_to_container, new_blob_name) + + # Copy blob from temp storage to permanent storage + try: + logging.info("Now copying file from temporary to permanent storage...") + logging.info("Source path: " + source_blob_path) + logging.info("Destination path: " + destination_blob_path) + blob_service.copy_blob(copy_to_container, new_blob_name, source_blob_path) + logging.info("Done.") + + # Add ImageId and permanent storage url to new dictionary to be sent to update function + update_urls_dictionary[image_id] = destination_blob_path + + # Delete the file from temp storage once it's been copied + logging.info("Now deleting image " + original_blob_name + " from temp storage container.") + try: + blob_service.delete_blob(copy_from_container, original_blob_name) + logging.info("Blob " + original_blob_name + " has been deleted successfully") + except Exception as e: + logging.error("ERROR: Deletion of blob " + original_blob_name + " failed. Exception: " + str(e)) + + except Exception as e: + logging.error("ERROR: Copy of blob " + original_blob_name + " failed. Exception: " + str(e)) + + return update_urls_dictionary From 245a86dd03af5fe984649de414ea233ce45d271d Mon Sep 17 00:00:00 2001 From: Amanda Kaufman Date: Wed, 14 Nov 2018 14:58:05 -0700 Subject: [PATCH 4/9] Dependency injection refactor. Renamed blob manipulation module and function. --- functions/pipeline/onboarding/__init__.py | 29 ++++++++++--------- .../__init__.py | 21 ++++---------- 2 files changed, 21 insertions(+), 29 deletions(-) rename functions/pipeline/shared/{onboarding_module => onboarding}/__init__.py (67%) diff --git a/functions/pipeline/onboarding/__init__.py b/functions/pipeline/onboarding/__init__.py index 2d2c6ad9..79b1aa87 100644 --- a/functions/pipeline/onboarding/__init__.py +++ b/functions/pipeline/onboarding/__init__.py @@ -4,7 +4,8 @@ from ..shared.db_provider import get_postgres_provider from ..shared.db_access import ImageTagDataAccess, ImageInfo -from ..shared.onboarding_module import blob_storage_manipulation +from ..shared.onboarding import copy_images_to_permanent_storage +from azure.storage.blob import BlockBlobService def main(req: func.HttpRequest) -> func.HttpResponse: logging.info('Python HTTP trigger function processed a request.') @@ -19,22 +20,23 @@ def main(req: func.HttpRequest) -> func.HttpResponse: logging.error(req.get_json()) raw_url_list = req_body["imageUrls"] except ValueError: - print("Unable to decode JSON body") - return func.HttpResponse("Unable to decode POST body", status_code=400) + logging.error("ERROR: Unable to decode JSON body") + return func.HttpResponse("ERROR: Unable to decode POST body", status_code=400) - # Build list of image objects to pass to DAL for insertion into DB. - image_object_list = [] - image_name_list = [] + if not raw_url_list: + return func.HttpResponse("ERROR: URL list empty.", status_code=401) # Check to ensure image URLs sent by client are all unique. url_list = set(raw_url_list) + # Build list of image objects to pass to DAL for insertion into DB. + image_object_list = [] + # TODO: Encapsulate this loop in a method # TODO: Wrap method in try/catch, send an appropriate http response in the event of an error for url in url_list: # Split original image name from URL original_filename = url.split("/")[-1] - image_name_list.append(original_filename) # Create ImageInfo object (def in db_access.py) # Note: For testing, default image height/width are set to 50x50 # TODO: Figure out where actual height/width need to come from @@ -51,22 +53,21 @@ def main(req: func.HttpRequest) -> func.HttpResponse: logging.error("ERROR: Database connection failed. Exception: " + str(e)) return func.HttpResponse("Unable to connect to database", status_code=503) - # Create user id + # Create/get user id user_id_number = data_access.create_user(user_id) logging.info("User id for {0} is {1}".format(user_id, str(user_id_number))) # Add new images to the database, and retrieve a dictionary ImageId's mapped to ImageUrl's image_id_url_map = data_access.add_new_images(image_object_list,user_id_number) - # Print out dictionary for debugging - logging.info("Image ID and URL map dictionary:") - logging.info(image_id_url_map) + copy_source = os.getenv('SOURCE_CONTAINER_NAME') + copy_destination = os.getenv('DESTINATION_CONTAINER_NAME') - copy_from_container = os.getenv('SOURCE_CONTAINER_NAME') - copy_to_container = os.getenv('DESTINATION_CONTAINER_NAME') + # Create blob service for storage account + blob_service = BlockBlobService(account_name=os.getenv('STORAGE_ACCOUNT_NAME'), account_key=os.getenv('STORAGE_ACCOUNT_KEY')) - update_urls_dictionary = blob_storage_manipulation(image_id_url_map, copy_from_container, copy_to_container) + update_urls_dictionary = copy_images_to_permanent_storage(image_id_url_map, copy_source, copy_destination, blob_service) logging.info("Now updating permanent URLs in the DB...") diff --git a/functions/pipeline/shared/onboarding_module/__init__.py b/functions/pipeline/shared/onboarding/__init__.py similarity index 67% rename from functions/pipeline/shared/onboarding_module/__init__.py rename to functions/pipeline/shared/onboarding/__init__.py index 70c4a165..28410539 100644 --- a/functions/pipeline/shared/onboarding_module/__init__.py +++ b/functions/pipeline/shared/onboarding/__init__.py @@ -2,22 +2,13 @@ import logging from azure.storage.blob import BlockBlobService -def blob_storage_manipulation(image_id_url_map, copy_from_container, copy_to_container): +def copy_images_to_permanent_storage(image_id_url_map, copy_source, copy_destination, blob_service): # Copy over images to permanent blob store and save URLs in a list update_urls_dictionary = {} - # Create blob service for storage account - blob_service = BlockBlobService(account_name=os.getenv('STORAGE_ACCOUNT_NAME'), account_key=os.getenv('STORAGE_ACCOUNT_KEY')) - - # TODO: Add check to make sure image exists in temp storage before attempting these operations - # TODO: Put blob storage manipulation into a separate function and add to shared codebase - # TODO: Try/catch to distinguish among errors + # Copy images from temporary to permanent storage and delete them. for key, value in image_id_url_map.items(): - # Verbose logging for testing - logging.info("Key: " + key) - logging.info("Value: " + str(value)) - original_image_url = key original_blob_name = original_image_url.split("/")[-1] file_extension = os.path.splitext(original_image_url)[1] @@ -30,15 +21,15 @@ def blob_storage_manipulation(image_id_url_map, copy_from_container, copy_to_con logging.info("New blob name: " + new_blob_name) # Create the blob URLs - source_blob_path = blob_service.make_blob_url(copy_from_container, original_blob_name) - destination_blob_path = blob_service.make_blob_url(copy_to_container, new_blob_name) + source_blob_path = blob_service.make_blob_url(copy_source, original_blob_name) + destination_blob_path = blob_service.make_blob_url(copy_destination, new_blob_name) # Copy blob from temp storage to permanent storage try: logging.info("Now copying file from temporary to permanent storage...") logging.info("Source path: " + source_blob_path) logging.info("Destination path: " + destination_blob_path) - blob_service.copy_blob(copy_to_container, new_blob_name, source_blob_path) + blob_service.copy_blob(copy_destination, new_blob_name, source_blob_path) logging.info("Done.") # Add ImageId and permanent storage url to new dictionary to be sent to update function @@ -47,7 +38,7 @@ def blob_storage_manipulation(image_id_url_map, copy_from_container, copy_to_con # Delete the file from temp storage once it's been copied logging.info("Now deleting image " + original_blob_name + " from temp storage container.") try: - blob_service.delete_blob(copy_from_container, original_blob_name) + blob_service.delete_blob(copy_source, original_blob_name) logging.info("Blob " + original_blob_name + " has been deleted successfully") except Exception as e: logging.error("ERROR: Deletion of blob " + original_blob_name + " failed. Exception: " + str(e)) From a32612fd091c2cdf55ccf7adff4e12feb92e7634 Mon Sep 17 00:00:00 2001 From: Amanda Kaufman Date: Wed, 14 Nov 2018 16:10:04 -0700 Subject: [PATCH 5/9] Changes --- functions/pipeline/shared/onboarding/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/functions/pipeline/shared/onboarding/__init__.py b/functions/pipeline/shared/onboarding/__init__.py index 28410539..f0643a3c 100644 --- a/functions/pipeline/shared/onboarding/__init__.py +++ b/functions/pipeline/shared/onboarding/__init__.py @@ -46,4 +46,7 @@ def copy_images_to_permanent_storage(image_id_url_map, copy_source, copy_destina except Exception as e: logging.error("ERROR: Copy of blob " + original_blob_name + " failed. Exception: " + str(e)) + # TODO: Modify this function to return a JSON string that separates those URLs which were successfully + # copied and those that failed. + return update_urls_dictionary From eb9a747ee89ef9b63461484b688ade47e1233c22 Mon Sep 17 00:00:00 2001 From: Amanda Kaufman Date: Thu, 15 Nov 2018 09:44:54 -0700 Subject: [PATCH 6/9] Fixed return status code for when an image copy/delete operation fails. Now returns 401 instead of 200. --- functions/pipeline/onboarding/__init__.py | 15 +++++++++------ functions/pipeline/shared/onboarding/__init__.py | 5 +++++ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/functions/pipeline/onboarding/__init__.py b/functions/pipeline/onboarding/__init__.py index 79b1aa87..c7233b62 100644 --- a/functions/pipeline/onboarding/__init__.py +++ b/functions/pipeline/onboarding/__init__.py @@ -70,13 +70,16 @@ def main(req: func.HttpRequest) -> func.HttpResponse: update_urls_dictionary = copy_images_to_permanent_storage(image_id_url_map, copy_source, copy_destination, blob_service) - logging.info("Now updating permanent URLs in the DB...") - data_access.update_image_urls(update_urls_dictionary, user_id_number) - logging.info("Done.") - # TODO: Return a JSON blob as a string containing list of successes and list of failures. If list of failures # contains any items, return a status code other than 200. - # Return string containing list of URLs to images in permanent blob storage - return func.HttpResponse("Images were successfully added to the database and copied to permanent storage.", status_code=200) + if not update_urls_dictionary: + return func.HttpResponse("ERROR: Image copy/delete operation failed. Check state of images in storage.", status_code=401) + else: + logging.info("Now updating permanent URLs in the DB...") + data_access.update_image_urls(update_urls_dictionary, user_id_number) + logging.info("Done.") + + # Return string containing list of URLs to images in permanent blob storage + return func.HttpResponse("Images were successfully added to the database and copied to permanent storage.", status_code=200) diff --git a/functions/pipeline/shared/onboarding/__init__.py b/functions/pipeline/shared/onboarding/__init__.py index f0643a3c..b7c647d6 100644 --- a/functions/pipeline/shared/onboarding/__init__.py +++ b/functions/pipeline/shared/onboarding/__init__.py @@ -42,11 +42,16 @@ def copy_images_to_permanent_storage(image_id_url_map, copy_source, copy_destina logging.info("Blob " + original_blob_name + " has been deleted successfully") except Exception as e: logging.error("ERROR: Deletion of blob " + original_blob_name + " failed. Exception: " + str(e)) + update_urls_dictionary.clear() + return update_urls_dictionary except Exception as e: logging.error("ERROR: Copy of blob " + original_blob_name + " failed. Exception: " + str(e)) + update_urls_dictionary.clear() + return update_urls_dictionary # TODO: Modify this function to return a JSON string that separates those URLs which were successfully # copied and those that failed. + # TODO: Alternative: return two lists, one "succeeded" list and one "failed" list return update_urls_dictionary From 25d549a2b03d88a903d6fee868424b61137b07d7 Mon Sep 17 00:00:00 2001 From: Amanda Kaufman Date: Thu, 15 Nov 2018 10:08:38 -0700 Subject: [PATCH 7/9] Wrapped code to build image object list into a function, and added try/catch in case of an error. --- functions/pipeline/onboarding/__init__.py | 41 +++++++++++++---------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/functions/pipeline/onboarding/__init__.py b/functions/pipeline/onboarding/__init__.py index c7233b62..a43728b0 100644 --- a/functions/pipeline/onboarding/__init__.py +++ b/functions/pipeline/onboarding/__init__.py @@ -29,22 +29,18 @@ def main(req: func.HttpRequest) -> func.HttpResponse: # Check to ensure image URLs sent by client are all unique. url_list = set(raw_url_list) - # Build list of image objects to pass to DAL for insertion into DB. - image_object_list = [] - - # TODO: Encapsulate this loop in a method - # TODO: Wrap method in try/catch, send an appropriate http response in the event of an error - for url in url_list: - # Split original image name from URL - original_filename = url.split("/")[-1] - # Create ImageInfo object (def in db_access.py) - # Note: For testing, default image height/width are set to 50x50 - # TODO: Figure out where actual height/width need to come from - image = ImageInfo(original_filename, url, 50, 50) - # Append image object to the list - image_object_list.append(image) + # Get list of image objects to pass to DAL for insertion into DB. + try: + image_object_list = build_objects_from_url_list(url_list) + # Logging for debugging and testing + logging.info("Image object list:") + for image in image_object_list: + logging.info(image.image_name) + except Exception as e: + logging.error("ERROR: Could not build image object list. Exception: " + str(e)) + return func.HttpResponse("ERROR: Could not build image object list.", status_code=401) - # TODO: Wrap db access section in try/catch, send an appropriate http response in the event of an error + # Connect to database. try: logging.info("Now connecting to database...") data_access = ImageTagDataAccess(get_postgres_provider()) @@ -66,10 +62,8 @@ def main(req: func.HttpRequest) -> func.HttpResponse: # Create blob service for storage account blob_service = BlockBlobService(account_name=os.getenv('STORAGE_ACCOUNT_NAME'), account_key=os.getenv('STORAGE_ACCOUNT_KEY')) - update_urls_dictionary = copy_images_to_permanent_storage(image_id_url_map, copy_source, copy_destination, blob_service) - # TODO: Return a JSON blob as a string containing list of successes and list of failures. If list of failures # contains any items, return a status code other than 200. @@ -83,3 +77,16 @@ def main(req: func.HttpRequest) -> func.HttpResponse: # Return string containing list of URLs to images in permanent blob storage return func.HttpResponse("Images were successfully added to the database and copied to permanent storage.", status_code=200) +# TODO: Wrap method in try/catch, send an appropriate http response in the event of an error +def build_objects_from_url_list(url_list): + image_object_list = [] + for url in url_list: + # Split original image name from URL + original_filename = url.split("/")[-1] + # Create ImageInfo object (def in db_access.py) + # Note: For now, default image height/width are set to 50x50 + # TODO: Figure out where actual height/width need to come from + image = ImageInfo(original_filename, url, 50, 50) + # Append image object to the list + image_object_list.append(image) + return image_object_list \ No newline at end of file From b9e69cc19eef92dbb2c3d844db062e2f1f9aa5c2 Mon Sep 17 00:00:00 2001 From: Amanda Kaufman Date: Thu, 15 Nov 2018 10:18:51 -0700 Subject: [PATCH 8/9] Code cleanup and more descriptive comments. --- functions/pipeline/onboarding/__init__.py | 20 ++++++++----------- .../pipeline/shared/onboarding/__init__.py | 8 ++------ 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/functions/pipeline/onboarding/__init__.py b/functions/pipeline/onboarding/__init__.py index a43728b0..c96142b2 100644 --- a/functions/pipeline/onboarding/__init__.py +++ b/functions/pipeline/onboarding/__init__.py @@ -32,10 +32,6 @@ def main(req: func.HttpRequest) -> func.HttpResponse: # Get list of image objects to pass to DAL for insertion into DB. try: image_object_list = build_objects_from_url_list(url_list) - # Logging for debugging and testing - logging.info("Image object list:") - for image in image_object_list: - logging.info(image.image_name) except Exception as e: logging.error("ERROR: Could not build image object list. Exception: " + str(e)) return func.HttpResponse("ERROR: Could not build image object list.", status_code=401) @@ -47,7 +43,7 @@ def main(req: func.HttpRequest) -> func.HttpResponse: logging.info("Connected.") except Exception as e: logging.error("ERROR: Database connection failed. Exception: " + str(e)) - return func.HttpResponse("Unable to connect to database", status_code=503) + return func.HttpResponse("ERROR: Unable to connect to database", status_code=503) # Create/get user id user_id_number = data_access.create_user(user_id) @@ -62,30 +58,30 @@ def main(req: func.HttpRequest) -> func.HttpResponse: # Create blob service for storage account blob_service = BlockBlobService(account_name=os.getenv('STORAGE_ACCOUNT_NAME'), account_key=os.getenv('STORAGE_ACCOUNT_KEY')) + # Copy images to permanent storage and get a dictionary of images for which to update URLs in DB. + # TODO: Prefer to have this function return a JSON blob as a string containing a list of successes + # and a list of failures. If the list of failures contains any items, return a status code other than 200. update_urls_dictionary = copy_images_to_permanent_storage(image_id_url_map, copy_source, copy_destination, blob_service) - # TODO: Return a JSON blob as a string containing list of successes and list of failures. If list of failures - # contains any items, return a status code other than 200. - + # If the dictionary of images is empty, this means a faiure occurred in a copy/delete operation. + # Otherwise, dictionary contains permanent image URLs for each image ID that was successfully copied. if not update_urls_dictionary: return func.HttpResponse("ERROR: Image copy/delete operation failed. Check state of images in storage.", status_code=401) else: logging.info("Now updating permanent URLs in the DB...") data_access.update_image_urls(update_urls_dictionary, user_id_number) logging.info("Done.") - # Return string containing list of URLs to images in permanent blob storage return func.HttpResponse("Images were successfully added to the database and copied to permanent storage.", status_code=200) -# TODO: Wrap method in try/catch, send an appropriate http response in the event of an error +# Given a list ofnimage URL's, build an ImageInfo object for each, and return a list of these image objects. def build_objects_from_url_list(url_list): image_object_list = [] for url in url_list: # Split original image name from URL original_filename = url.split("/")[-1] # Create ImageInfo object (def in db_access.py) - # Note: For now, default image height/width are set to 50x50 - # TODO: Figure out where actual height/width need to come from + # TODO: Figure out where actual height/width need to come from. Values are hard-coded for testing. image = ImageInfo(original_filename, url, 50, 50) # Append image object to the list image_object_list.append(image) diff --git a/functions/pipeline/shared/onboarding/__init__.py b/functions/pipeline/shared/onboarding/__init__.py index b7c647d6..a73ae75a 100644 --- a/functions/pipeline/shared/onboarding/__init__.py +++ b/functions/pipeline/shared/onboarding/__init__.py @@ -2,13 +2,13 @@ import logging from azure.storage.blob import BlockBlobService +# TODO: Modify this function to return a JSON string that contains a "succeeded" list and a "failed" list. def copy_images_to_permanent_storage(image_id_url_map, copy_source, copy_destination, blob_service): - # Copy over images to permanent blob store and save URLs in a list + # Create a dictionary to store map of new permanent image URLs to image ID's update_urls_dictionary = {} # Copy images from temporary to permanent storage and delete them. for key, value in image_id_url_map.items(): - original_image_url = key original_blob_name = original_image_url.split("/")[-1] file_extension = os.path.splitext(original_image_url)[1] @@ -50,8 +50,4 @@ def copy_images_to_permanent_storage(image_id_url_map, copy_source, copy_destina update_urls_dictionary.clear() return update_urls_dictionary - # TODO: Modify this function to return a JSON string that separates those URLs which were successfully - # copied and those that failed. - # TODO: Alternative: return two lists, one "succeeded" list and one "failed" list - return update_urls_dictionary From 21fe5ff05f1e82573162cc456ba2755daff99c52 Mon Sep 17 00:00:00 2001 From: Amanda Kaufman Date: Thu, 15 Nov 2018 13:51:34 -0700 Subject: [PATCH 9/9] PR comment - deleted unnecessary module import. --- functions/pipeline/shared/onboarding/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/functions/pipeline/shared/onboarding/__init__.py b/functions/pipeline/shared/onboarding/__init__.py index a73ae75a..59b814c3 100644 --- a/functions/pipeline/shared/onboarding/__init__.py +++ b/functions/pipeline/shared/onboarding/__init__.py @@ -1,6 +1,5 @@ import os import logging -from azure.storage.blob import BlockBlobService # TODO: Modify this function to return a JSON string that contains a "succeeded" list and a "failed" list. def copy_images_to_permanent_storage(image_id_url_map, copy_source, copy_destination, blob_service):