From 8608f69e76f9daebdbe2923db77a4c011a7e63e7 Mon Sep 17 00:00:00 2001 From: Michael Richardson Date: Wed, 10 Feb 2021 15:39:34 -0800 Subject: [PATCH 1/5] update azure blob SDK version --- .../botbuilder/azure/blob_storage.py | 185 +++++++++++++----- libraries/botbuilder-azure/setup.py | 2 +- .../tests/test_blob_storage.py | 14 +- 3 files changed, 144 insertions(+), 57 deletions(-) diff --git a/libraries/botbuilder-azure/botbuilder/azure/blob_storage.py b/libraries/botbuilder-azure/botbuilder/azure/blob_storage.py index b69217680..ebf46bbe1 100644 --- a/libraries/botbuilder-azure/botbuilder/azure/blob_storage.py +++ b/libraries/botbuilder-azure/botbuilder/azure/blob_storage.py @@ -3,13 +3,33 @@ from jsonpickle import encode from jsonpickle.unpickler import Unpickler -from azure.storage.blob import BlockBlobService, Blob, PublicAccess +from azure.core.exceptions import ( + HttpResponseError, + ResourceExistsError, + ResourceNotFoundError, +) from botbuilder.core import Storage - -# TODO: sanitize_blob_name +from azure.storage.blob.aio import ( + BlobServiceClient, + BlobClient, + StorageStreamDownloader, +) +from azure.core import MatchConditions class BlobStorageSettings: + """The class for Azure Blob configuration for the Azure Bot Framework. + + :param container_name: Name of the Blob container. + :type container_name: str + :param account_name: Name of the Blob Storage account. Required if not using connection_string. + :type account_name: str + :param account_key: Key of the Blob Storage account. Required if not using connection_string. + :type account_key: str + :param connection_string: Connection string of the Blob Storage account. + Required if not using account_name and account_key. + :type connection_string: str + """ def __init__( self, container_name: str, @@ -23,56 +43,106 @@ def __init__( self.connection_string = connection_string +# New Azure Blob SDK only allows connection strings, but our SDK allows key+name. +# This is here for backwards compatibility. +def convert_account_name_and_key_to_connection_string(settings: BlobStorageSettings): + if not settings.account_name or not settings.account_key: + raise Exception( + "account_name and account_key are both required for BlobStorageSettings if not using a connections string." + ) + return ( + f"DefaultEndpointsProtocol=https;AccountName={settings.account_name};" + f"AccountKey={settings.account_key};EndpointSuffix=core.windows.net" + ) + + class BlobStorage(Storage): + """An Azure Blob based storage provider for a bot. + + This class uses a single Azure Storage Blob Container. + Each entity or StoreItem is serialized into a JSON string and stored in an individual text blob. + Each blob is named after the store item key, which is encoded so that it conforms a valid blob name. + If an entity is an StoreItem, the storage object will set the entity's e_tag + property value to the blob's e_tag upon read. Afterward, an match_condition with the ETag value + will be generated during Write. New entities start with a null e_tag. + + :param settings: Settings used to instantiate the Blob service. + :type settings: :class:`botbuilder.azure.BlobStorageSettings` + """ def __init__(self, settings: BlobStorageSettings): + if not settings.container_name: + raise Exception("Container name is required.") + if settings.connection_string: - client = BlockBlobService(connection_string=settings.connection_string) - elif settings.account_name and settings.account_key: - client = BlockBlobService( - account_name=settings.account_name, account_key=settings.account_key + blob_service_client = BlobServiceClient.from_connection_string( + settings.connection_string ) else: - raise Exception( - "Connection string should be provided if there are no account name and key" + blob_service_client = BlobServiceClient.from_connection_string( + convert_account_name_and_key_to_connection_string(settings) ) - self.client = client - self.settings = settings + self.__container_client = blob_service_client.get_container_client( + settings.container_name + ) + + self.__initialized = False + + async def _initialize(self): + if self.__initialized is False: + # This should only happen once - assuming this is a singleton. + # ContainerClient.exists() method is available in an unreleased version of the SDK. Until then, we use: + try: + await self.__container_client.create_container() + except ResourceExistsError: + pass + self.__initialized = True + return self.__initialized async def read(self, keys: List[str]) -> Dict[str, object]: + """Retrieve entities from the configured blob container. + + :param keys: An array of entity keys. + :type keys: Dict[str, object] + :return dict: + """ if not keys: raise Exception("Keys are required when reading") - self.client.create_container(self.settings.container_name) - self.client.set_container_acl( - self.settings.container_name, public_access=PublicAccess.Container - ) + await self._initialize() + items = {} for key in keys: - if self.client.exists( - container_name=self.settings.container_name, blob_name=key - ): - items[key] = self._blob_to_store_item( - self.client.get_blob_to_text( - container_name=self.settings.container_name, blob_name=key - ) - ) + blob_name = self._get_blob_name(key) + blob_client = self.__container_client.get_blob_client(blob_name) + + try: + items[key] = await self._inner_read_blob(blob_client) + except HttpResponseError as err: + if err.status_code == 404: + continue return items async def write(self, changes: Dict[str, object]): + """Stores a new entity in the configured blob container. + + :param changes: The changes to write to storage. + :type changes: Dict[str, object] + :return: + """ if changes is None: raise Exception("Changes are required when writing") if not changes: return - self.client.create_container(self.settings.container_name) - self.client.set_container_acl( - self.settings.container_name, public_access=PublicAccess.Container - ) + await self._initialize() for (name, item) in changes.items(): + blob_name = self._get_blob_name(name) + blob_reference = self.__container_client.get_blob_client(blob_name) + e_tag = None if isinstance(item, dict): e_tag = item.get("e_tag", None) @@ -81,39 +151,54 @@ async def write(self, changes: Dict[str, object]): e_tag = None if e_tag == "*" else e_tag if e_tag == "": raise Exception("blob_storage.write(): etag missing") + item_str = self._store_item_to_str(item) - try: - self.client.create_blob_from_text( - container_name=self.settings.container_name, - blob_name=name, - text=item_str, - if_match=e_tag, + + if e_tag: + await blob_reference.upload_blob( + item_str, match_condition=MatchConditions.IfNotModified, etag=e_tag ) - except Exception as error: - raise error + else: + await blob_reference.upload_blob(item_str, overwrite=True) async def delete(self, keys: List[str]): + """Deletes entity blobs from the configured container. + + :param keys: An array of entity keys. + :type keys: Dict[str, object] + """ if keys is None: raise Exception("BlobStorage.delete: keys parameter can't be null") - self.client.create_container(self.settings.container_name) - self.client.set_container_acl( - self.settings.container_name, public_access=PublicAccess.Container - ) + await self._initialize() for key in keys: - if self.client.exists( - container_name=self.settings.container_name, blob_name=key - ): - self.client.delete_blob( - container_name=self.settings.container_name, blob_name=key - ) + blob_name = self._get_blob_name(key) + blob_client = self.__container_client.get_blob_client(blob_name) + try: + await blob_client.delete_blob() + # We can't delete what's already gone. + except ResourceNotFoundError: + pass - def _blob_to_store_item(self, blob: Blob) -> object: - item = json.loads(blob.content) - item["e_tag"] = blob.properties.etag + def _store_item_to_str(self, item: object) -> str: + return encode(item) + + async def _inner_read_blob(self, blob_client: BlobClient): + blob = await blob_client.download_blob() + + return await self._blob_to_store_item(blob) + + @staticmethod + async def _blob_to_store_item(blob: StorageStreamDownloader) -> object: + item = json.loads(await blob.content_as_text()) + item["e_tag"] = blob.properties.etag.replace('"', "") result = Unpickler().restore(item) return result - def _store_item_to_str(self, item: object) -> str: - return encode(item) + @staticmethod + def _get_blob_name(key: str): + if not key: + raise Exception("Key required.") + + return encode(key) diff --git a/libraries/botbuilder-azure/setup.py b/libraries/botbuilder-azure/setup.py index 48333f8d3..165800f3d 100644 --- a/libraries/botbuilder-azure/setup.py +++ b/libraries/botbuilder-azure/setup.py @@ -6,7 +6,7 @@ REQUIRES = [ "azure-cosmos==3.2.0", - "azure-storage-blob==2.1.0", + "azure-storage-blob==12.7.0", "botbuilder-schema==4.12.0", "botframework-connector==4.12.0", "jsonpickle==1.2", diff --git a/libraries/botbuilder-azure/tests/test_blob_storage.py b/libraries/botbuilder-azure/tests/test_blob_storage.py index 31f54a231..b6421b93a 100644 --- a/libraries/botbuilder-azure/tests/test_blob_storage.py +++ b/libraries/botbuilder-azure/tests/test_blob_storage.py @@ -5,6 +5,8 @@ from botbuilder.core import StoreItem from botbuilder.azure import BlobStorage, BlobStorageSettings from botbuilder.testing import StorageBaseTests +from azure.storage.blob.aio import BlobServiceClient +from azure.core.exceptions import ResourceNotFoundError # local blob emulator instance blob @@ -26,12 +28,12 @@ def get_storage(): async def reset(): - storage = get_storage() + storage = BlobServiceClient.from_connection_string( + BLOB_STORAGE_SETTINGS.connection_string + ) try: - await storage.client.delete_container( - container_name=BLOB_STORAGE_SETTINGS.container_name - ) - except Exception: + await storage.delete_container(BLOB_STORAGE_SETTINGS.container_name) + except ResourceNotFoundError: pass @@ -44,7 +46,7 @@ def __init__(self, counter=1, e_tag="*"): class TestBlobStorageConstructor: @pytest.mark.asyncio - async def test_blob_storage_init_should_error_without_cosmos_db_config(self): + async def test_blob_storage_init_should_error_without_blob_config(self): try: BlobStorage(BlobStorageSettings()) # pylint: disable=no-value-for-parameter except Exception as error: From 52fb59c964d9704f365db1c41a10b651306d63b0 Mon Sep 17 00:00:00 2001 From: Michael Richardson Date: Wed, 10 Feb 2021 15:51:33 -0800 Subject: [PATCH 2/5] black fixes --- libraries/botbuilder-azure/botbuilder/azure/blob_storage.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libraries/botbuilder-azure/botbuilder/azure/blob_storage.py b/libraries/botbuilder-azure/botbuilder/azure/blob_storage.py index ebf46bbe1..6cd6f8f3d 100644 --- a/libraries/botbuilder-azure/botbuilder/azure/blob_storage.py +++ b/libraries/botbuilder-azure/botbuilder/azure/blob_storage.py @@ -30,6 +30,7 @@ class BlobStorageSettings: Required if not using account_name and account_key. :type connection_string: str """ + def __init__( self, container_name: str, @@ -69,6 +70,7 @@ class BlobStorage(Storage): :param settings: Settings used to instantiate the Blob service. :type settings: :class:`botbuilder.azure.BlobStorageSettings` """ + def __init__(self, settings: BlobStorageSettings): if not settings.container_name: raise Exception("Container name is required.") From cc9f206232ef5b07fc7bf873e24d29e112d06e43 Mon Sep 17 00:00:00 2001 From: Michael Richardson Date: Wed, 10 Feb 2021 16:11:35 -0800 Subject: [PATCH 3/5] pylint fixes --- libraries/botbuilder-azure/botbuilder/azure/blob_storage.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/botbuilder-azure/botbuilder/azure/blob_storage.py b/libraries/botbuilder-azure/botbuilder/azure/blob_storage.py index 6cd6f8f3d..02b13eda2 100644 --- a/libraries/botbuilder-azure/botbuilder/azure/blob_storage.py +++ b/libraries/botbuilder-azure/botbuilder/azure/blob_storage.py @@ -3,18 +3,18 @@ from jsonpickle import encode from jsonpickle.unpickler import Unpickler +from azure.core import MatchConditions from azure.core.exceptions import ( HttpResponseError, ResourceExistsError, ResourceNotFoundError, ) -from botbuilder.core import Storage from azure.storage.blob.aio import ( BlobServiceClient, BlobClient, StorageStreamDownloader, ) -from azure.core import MatchConditions +from botbuilder.core import Storage class BlobStorageSettings: From 5081fdcd473279c32c154b2cfc55130ce18a7780 Mon Sep 17 00:00:00 2001 From: Michael Richardson Date: Wed, 10 Feb 2021 16:18:33 -0800 Subject: [PATCH 4/5] more pylint fixes --- libraries/botbuilder-azure/tests/test_blob_storage.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/botbuilder-azure/tests/test_blob_storage.py b/libraries/botbuilder-azure/tests/test_blob_storage.py index b6421b93a..6357d31d6 100644 --- a/libraries/botbuilder-azure/tests/test_blob_storage.py +++ b/libraries/botbuilder-azure/tests/test_blob_storage.py @@ -2,11 +2,11 @@ # Licensed under the MIT License. import pytest +from azure.core.exceptions import ResourceNotFoundError +from azure.storage.blob.aio import BlobServiceClient from botbuilder.core import StoreItem from botbuilder.azure import BlobStorage, BlobStorageSettings from botbuilder.testing import StorageBaseTests -from azure.storage.blob.aio import BlobServiceClient -from azure.core.exceptions import ResourceNotFoundError # local blob emulator instance blob From 4955ee99f20c2a5d82b79cfaa71f85752d09bb86 Mon Sep 17 00:00:00 2001 From: Michael Richardson Date: Thu, 11 Feb 2021 11:36:21 -0800 Subject: [PATCH 5/5] don't try to encode blob name --- .../botbuilder/azure/blob_storage.py | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/libraries/botbuilder-azure/botbuilder/azure/blob_storage.py b/libraries/botbuilder-azure/botbuilder/azure/blob_storage.py index 02b13eda2..808105209 100644 --- a/libraries/botbuilder-azure/botbuilder/azure/blob_storage.py +++ b/libraries/botbuilder-azure/botbuilder/azure/blob_storage.py @@ -116,8 +116,7 @@ async def read(self, keys: List[str]) -> Dict[str, object]: items = {} for key in keys: - blob_name = self._get_blob_name(key) - blob_client = self.__container_client.get_blob_client(blob_name) + blob_client = self.__container_client.get_blob_client(key) try: items[key] = await self._inner_read_blob(blob_client) @@ -142,8 +141,7 @@ async def write(self, changes: Dict[str, object]): await self._initialize() for (name, item) in changes.items(): - blob_name = self._get_blob_name(name) - blob_reference = self.__container_client.get_blob_client(blob_name) + blob_reference = self.__container_client.get_blob_client(name) e_tag = None if isinstance(item, dict): @@ -175,8 +173,7 @@ async def delete(self, keys: List[str]): await self._initialize() for key in keys: - blob_name = self._get_blob_name(key) - blob_client = self.__container_client.get_blob_client(blob_name) + blob_client = self.__container_client.get_blob_client(key) try: await blob_client.delete_blob() # We can't delete what's already gone. @@ -197,10 +194,3 @@ async def _blob_to_store_item(blob: StorageStreamDownloader) -> object: item["e_tag"] = blob.properties.etag.replace('"', "") result = Unpickler().restore(item) return result - - @staticmethod - def _get_blob_name(key: str): - if not key: - raise Exception("Key required.") - - return encode(key)