From f80c915a8a9cb7d310f42e5719d34e6378332250 Mon Sep 17 00:00:00 2001 From: hallvictoria Date: Thu, 2 May 2024 12:58:14 -0500 Subject: [PATCH 1/3] unique cache for fx name --- azure_functions_worker/bindings/meta.py | 21 +++++++++++---- azure_functions_worker/dispatcher.py | 1 + .../function_app.py | 14 ++++++++-- .../test_deferred_bindings.py | 3 ++- .../test_deferred_bindings_blob_functions.py | 26 ++++++++++++++++--- 5 files changed, 54 insertions(+), 11 deletions(-) diff --git a/azure_functions_worker/bindings/meta.py b/azure_functions_worker/bindings/meta.py index c6148344d..f312da06a 100644 --- a/azure_functions_worker/bindings/meta.py +++ b/azure_functions_worker/bindings/meta.py @@ -156,6 +156,7 @@ def from_incoming_proto( pytype: typing.Optional[type], trigger_metadata: typing.Optional[typing.Dict[str, protos.TypedData]], shmem_mgr: SharedMemoryManager, + function_name: str, is_deferred_binding: typing.Optional[bool] = False) -> typing.Any: binding = get_binding(binding, is_deferred_binding) if trigger_metadata: @@ -184,7 +185,8 @@ def from_incoming_proto( pb=pb, pytype=pytype, datum=datum, - metadata=metadata) + metadata=metadata, + function_name=function_name) return binding.decode(datum, trigger_metadata=metadata) except NotImplementedError: # Binding does not support the data. @@ -281,29 +283,38 @@ def deferred_bindings_decode(binding: typing.Any, pb: protos.ParameterBinding, *, pytype: typing.Optional[type], datum: typing.Any, - metadata: typing.Any): + metadata: typing.Any, + function_name: str): """ This cache holds deferred binding types (ie. BlobClient, ContainerClient) That have already been created, so that the worker can reuse the Previously created type without creating a new one. + For async types, the function_name is needed as a key to differentiate. + This prevents a known SDK issue where reusing a client across functions + can lose the session context and cause an error. + If cache is empty or key doesn't exist, deferred_binding_type is None """ global deferred_bindings_cache if deferred_bindings_cache.get((pb.name, pytype, - datum.value.content), None) is not None: + datum.value.content, + function_name), None) is not None: return deferred_bindings_cache.get((pb.name, pytype, - datum.value.content)) + datum.value.content, + function_name)) else: deferred_binding_type = binding.decode(datum, trigger_metadata=metadata, pytype=pytype) + deferred_bindings_cache[(pb.name, pytype, - datum.value.content)] = deferred_binding_type + datum.value.content, + function_name)] = deferred_binding_type return deferred_binding_type diff --git a/azure_functions_worker/dispatcher.py b/azure_functions_worker/dispatcher.py index 902e9d86e..003e0a036 100644 --- a/azure_functions_worker/dispatcher.py +++ b/azure_functions_worker/dispatcher.py @@ -568,6 +568,7 @@ async def _handle__invocation_request(self, request): trigger_metadata=trigger_metadata, pytype=pb_type_info.pytype, shmem_mgr=self._shmem_mgr, + function_name=self._functions.get_function(function_id).name, is_deferred_binding=pb_type_info.deferred_bindings_enabled) http_v2_enabled = self._functions.get_function(function_id) \ diff --git a/tests/extension_tests/deferred_bindings_tests/deferred_bindings_blob_functions/function_app.py b/tests/extension_tests/deferred_bindings_tests/deferred_bindings_blob_functions/function_app.py index 4df62c480..1efc4151b 100644 --- a/tests/extension_tests/deferred_bindings_tests/deferred_bindings_blob_functions/function_app.py +++ b/tests/extension_tests/deferred_bindings_tests/deferred_bindings_blob_functions/function_app.py @@ -255,5 +255,15 @@ def put_blob_bytes(req: func.HttpRequest, file: func.Out[bytes]) -> str: connection="AzureWebJobsStorage") @app.route(route="blob_cache") def blob_cache(req: func.HttpRequest, - client: blob.BlobClient) -> str: - return client.download_blob(encoding='utf-8').readall() + client: blob.BlobClient) -> func.HttpResponse: + return func.HttpResponse(repr(client)) + + +@app.function_name(name="blob_cache2") +@app.blob_input(arg_name="client", + path="python-worker-tests/test-blobclient-triggered.txt", + connection="AzureWebJobsStorage") +@app.route(route="blob_cache2") +def blob_cache2(req: func.HttpRequest, + client: blob.BlobClient) -> func.HttpResponse: + return func.HttpResponse(repr(client)) diff --git a/tests/extension_tests/deferred_bindings_tests/test_deferred_bindings.py b/tests/extension_tests/deferred_bindings_tests/test_deferred_bindings.py index ab614a55c..3510104aa 100644 --- a/tests/extension_tests/deferred_bindings_tests/test_deferred_bindings.py +++ b/tests/extension_tests/deferred_bindings_tests/test_deferred_bindings.py @@ -100,7 +100,8 @@ def test_deferred_bindings_decode(self): datum = datumdef.Datum(value=sample_mbd, type='model_binding_data') obj = meta.deferred_bindings_decode(binding=binding, pb=pb, - pytype=BlobClient, datum=datum, metadata={}) + pytype=BlobClient, datum=datum, metadata={}, + function_name="test_function") self.assertIsNotNone(obj) diff --git a/tests/extension_tests/deferred_bindings_tests/test_deferred_bindings_blob_functions.py b/tests/extension_tests/deferred_bindings_tests/test_deferred_bindings_blob_functions.py index 54e36f9f1..6446e27eb 100644 --- a/tests/extension_tests/deferred_bindings_tests/test_deferred_bindings_blob_functions.py +++ b/tests/extension_tests/deferred_bindings_tests/test_deferred_bindings_blob_functions.py @@ -5,7 +5,6 @@ import sys from tests.utils import testutils -from azure_functions_worker.bindings import meta @unittest.skipIf(sys.version_info.minor <= 8, "The base extension" @@ -169,13 +168,34 @@ def test_type_undefined(self): self.assertEqual(r.text, 'test-data') def test_caching(self): - # Cache is empty at the start - self.assertEqual(meta.deferred_bindings_cache, {}) + ''' + The cache returns the same type based on resource and function name. + Two different functions with clients that access the same resource + will have two different clients. This tests that the same client + is returned for each invocation and that the clients are different + between the two functions. + ''' + r = self.webhost.request('GET', 'blob_cache') + r2 = self.webhost.request('GET', 'blob_cache2') self.assertEqual(r.status_code, 200) + self.assertEqual(r2.status_code, 200) + client = r.text + client2 = r2.text + self.assertNotEqual(client, client2) r = self.webhost.request('GET', 'blob_cache') + r2 = self.webhost.request('GET', 'blob_cache2') self.assertEqual(r.status_code, 200) + self.assertEqual(r2.status_code, 200) + self.assertEqual(r.text, client) + self.assertEqual(r2.text, client2) + self.assertNotEqual(r.text, r2.text) r = self.webhost.request('GET', 'blob_cache') + r2 = self.webhost.request('GET', 'blob_cache2') self.assertEqual(r.status_code, 200) + self.assertEqual(r2.status_code, 200) + self.assertEqual(r.text, client) + self.assertEqual(r2.text, client2) + self.assertNotEqual(r.text, r2.text) From b08ff9a08ba160202f88dbed63068f7d32dc83b8 Mon Sep 17 00:00:00 2001 From: hallvictoria Date: Thu, 2 May 2024 13:15:37 -0500 Subject: [PATCH 2/3] lint --- azure_functions_worker/bindings/meta.py | 2 ++ azure_functions_worker/dispatcher.py | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/azure_functions_worker/bindings/meta.py b/azure_functions_worker/bindings/meta.py index f312da06a..8a7db4852 100644 --- a/azure_functions_worker/bindings/meta.py +++ b/azure_functions_worker/bindings/meta.py @@ -294,6 +294,8 @@ def deferred_bindings_decode(binding: typing.Any, This prevents a known SDK issue where reusing a client across functions can lose the session context and cause an error. + The cache key is based on: param name, type, resource, function_name + If cache is empty or key doesn't exist, deferred_binding_type is None """ global deferred_bindings_cache diff --git a/azure_functions_worker/dispatcher.py b/azure_functions_worker/dispatcher.py index 003e0a036..ac188165d 100644 --- a/azure_functions_worker/dispatcher.py +++ b/azure_functions_worker/dispatcher.py @@ -568,7 +568,8 @@ async def _handle__invocation_request(self, request): trigger_metadata=trigger_metadata, pytype=pb_type_info.pytype, shmem_mgr=self._shmem_mgr, - function_name=self._functions.get_function(function_id).name, + function_name=self._functions.get_function( + function_id).name, is_deferred_binding=pb_type_info.deferred_bindings_enabled) http_v2_enabled = self._functions.get_function(function_id) \ From d7214606a8b17504169ee772e9084c4aeda96548 Mon Sep 17 00:00:00 2001 From: Victoria Hall Date: Wed, 10 Jul 2024 15:09:39 -0500 Subject: [PATCH 3/3] added test for same resource different binding --- .../function_app.py | 15 +++++++++++++ .../test_deferred_bindings_blob_functions.py | 21 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/tests/extension_tests/deferred_bindings_tests/deferred_bindings_blob_functions/function_app.py b/tests/extension_tests/deferred_bindings_tests/deferred_bindings_blob_functions/function_app.py index 7e93a0bcc..075d8a78a 100644 --- a/tests/extension_tests/deferred_bindings_tests/deferred_bindings_blob_functions/function_app.py +++ b/tests/extension_tests/deferred_bindings_tests/deferred_bindings_blob_functions/function_app.py @@ -269,6 +269,21 @@ def blob_cache2(req: func.HttpRequest, return func.HttpResponse(repr(cachedClient)) +@app.function_name(name="blob_cache3") +@app.blob_input(arg_name="cachedClient", + path="python-worker-tests/test-blobclient-triggered.txt", + connection="AzureWebJobsStorage") +@app.blob_input(arg_name="cachedClient2", + path="python-worker-tests/test-blobclient-triggered.txt", + connection="AzureWebJobsStorage") +@app.route(route="blob_cache3") +def blob_cache3(req: func.HttpRequest, + cachedClient: blob.BlobClient, + cachedClient2: blob.BlobClient) -> func.HttpResponse: + return func.HttpResponse("Client 1: " + repr(cachedClient) + + " | Client 2: " + repr(cachedClient2)) + + @app.function_name(name="invalid_connection_info") @app.blob_input(arg_name="client", path="python-worker-tests/test-blobclient-triggered.txt", diff --git a/tests/extension_tests/deferred_bindings_tests/test_deferred_bindings_blob_functions.py b/tests/extension_tests/deferred_bindings_tests/test_deferred_bindings_blob_functions.py index 7ceb03650..1ab83cb19 100644 --- a/tests/extension_tests/deferred_bindings_tests/test_deferred_bindings_blob_functions.py +++ b/tests/extension_tests/deferred_bindings_tests/test_deferred_bindings_blob_functions.py @@ -204,6 +204,27 @@ def test_caching(self): self.assertEqual(r2.text, client2) self.assertNotEqual(r.text, r2.text) + def test_caching_same_resource(self): + ''' + The cache returns the same type based on param name. + One functions with two clients that access the same resource + will have two different clients. This tests that the same clients + are returned for each invocation and that the clients are different + between the two bindings. + ''' + + r = self.webhost.request('GET', 'blob_cache3') + self.assertEqual(r.status_code, 200) + clients = r.text.split(" | ") + self.assertNotEqual(clients[0], clients[1]) + + r2 = self.webhost.request('GET', 'blob_cache3') + self.assertEqual(r2.status_code, 200) + clients_second_call = r2.text.split(" | ") + self.assertEqual(clients[0], clients_second_call[0]) + self.assertEqual(clients[1], clients_second_call[1]) + self.assertNotEqual(clients_second_call[0], clients_second_call[1]) + def test_failed_client_creation(self): r = self.webhost.request('GET', 'invalid_connection_info') # Without the http_v2_enabled default definition, this request would time out.