From 143afe4d64c7ea57f24d938aec534dc86717b2fd Mon Sep 17 00:00:00 2001 From: "Hanzhang Zeng (Roger)" Date: Wed, 15 Jul 2020 16:40:33 -0700 Subject: [PATCH 1/3] Add sys.path reload in Linux Consumption --- azure_functions_worker/dispatcher.py | 9 +++++++-- python/prodV2/worker.py | 9 +++++++++ python/prodV3/worker.py | 9 +++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/azure_functions_worker/dispatcher.py b/azure_functions_worker/dispatcher.py index cc4bfb46b..b72c11db0 100644 --- a/azure_functions_worker/dispatcher.py +++ b/azure_functions_worker/dispatcher.py @@ -399,6 +399,7 @@ async def _handle__invocation_request(self, req): exception=self._serialize_exception(ex)))) async def _handle__function_environment_reload_request(self, req): + '''Only runs on Linux Consumption placeholder specialization.''' try: logger.info('Received FunctionEnvironmentReloadRequest, ' 'request ID: %s', self.request_id) @@ -410,12 +411,16 @@ async def _handle__function_environment_reload_request(self, req): # customer use import azure.functions # NoQA + # Append function project root to module finding sys.path + if func_env_reload_request.function_app_directory: + sys.path.append(func_env_reload_request.function_app_directory) + + # Clear sys.path import cache, reload all module from new sys.path sys.path_importer_cache.clear() + # Reload environment variables os.environ.clear() - env_vars = func_env_reload_request.environment_variables - for var in env_vars: os.environ[var] = env_vars[var] diff --git a/python/prodV2/worker.py b/python/prodV2/worker.py index 57848d767..da084df5b 100644 --- a/python/prodV2/worker.py +++ b/python/prodV2/worker.py @@ -13,6 +13,7 @@ # Azure environment variables AZURE_WEBSITE_INSTANCE_ID = "WEBSITE_INSTANCE_ID" AZURE_CONTAINER_NAME = "CONTAINER_NAME" +AZURE_WEBJOBS_SCRIPT_ROOT = "AzureWebJobsScriptRoot" def is_azure_environment(): @@ -20,6 +21,13 @@ def is_azure_environment(): or AZURE_WEBSITE_INSTANCE_ID in os.environ) +def add_script_root_to_sys_path(): + '''Append function project root to module finding sys.path''' + functions_script_root = os.getenv(AZURE_WEBJOBS_SCRIPT_ROOT) + if functions_script_root is not None: + sys.path.append(functions_script_root) + + def determine_user_pkg_paths(): minor_version = sys.version_info[1] @@ -56,6 +64,7 @@ def determine_user_pkg_paths(): env) else: sys.path.insert(1, func_worker_dir) + add_script_root_to_sys_path() from azure_functions_worker import main main.main() diff --git a/python/prodV3/worker.py b/python/prodV3/worker.py index 60ea074f2..d5d78fbb3 100644 --- a/python/prodV3/worker.py +++ b/python/prodV3/worker.py @@ -13,6 +13,7 @@ # Azure environment variables AZURE_WEBSITE_INSTANCE_ID = "WEBSITE_INSTANCE_ID" AZURE_CONTAINER_NAME = "CONTAINER_NAME" +AZURE_WEBJOBS_SCRIPT_ROOT = "AzureWebJobsScriptRoot" def is_azure_environment(): @@ -20,6 +21,13 @@ def is_azure_environment(): or AZURE_WEBSITE_INSTANCE_ID in os.environ) +def add_script_root_to_sys_path(): + '''Append function project root to module finding sys.path''' + functions_script_root = os.getenv(AZURE_WEBJOBS_SCRIPT_ROOT) + if functions_script_root is not None: + sys.path.append(functions_script_root) + + def determine_user_pkg_paths(): minor_version = sys.version_info[1] @@ -56,6 +64,7 @@ def determine_user_pkg_paths(): env) else: sys.path.insert(1, func_worker_dir) + add_script_root_to_sys_path() from azure_functions_worker import main main.main() From 42a85b3ced35362570bee68c68556661dfaf0950 Mon Sep 17 00:00:00 2001 From: "Hanzhang Zeng (Roger)" Date: Wed, 15 Jul 2020 19:52:13 -0700 Subject: [PATCH 2/3] Add comments --- python/prodV2/worker.py | 16 ++++++++++++++++ python/prodV3/worker.py | 16 ++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/python/prodV2/worker.py b/python/prodV2/worker.py index da084df5b..a57848809 100644 --- a/python/prodV2/worker.py +++ b/python/prodV2/worker.py @@ -17,6 +17,7 @@ def is_azure_environment(): + '''Check if the function app is running on the cloud''' return (AZURE_CONTAINER_NAME in os.environ or AZURE_WEBSITE_INSTANCE_ID in os.environ) @@ -29,6 +30,16 @@ def add_script_root_to_sys_path(): def determine_user_pkg_paths(): + '''This finds the user packages when function apps are running on the cloud + + For Python 3.6 app, the third-party packages can live in any of the paths: + /home/site/wwwroot/.python_packages/lib/site-packages + /home/site/wwwroot/.python_packages/lib/python3.6/site-packages + /home/site/wwwroot/worker_venv/lib/python3.6/site-packages + + For Python 3.7, we only accept: + /home/site/wwwroot/.python_packages/lib/site-packages + ''' minor_version = sys.version_info[1] home = Path.home() @@ -57,12 +68,17 @@ def determine_user_pkg_paths(): user_pkg_paths = determine_user_pkg_paths() joined_pkg_paths = os.pathsep.join(user_pkg_paths) + + # On cloud, we prioritize third-party user packages + # over worker packages in PYTHONPATH env['PYTHONPATH'] = f'{joined_pkg_paths}:{func_worker_dir}' os.execve(sys.executable, [sys.executable, '-m', 'azure_functions_worker'] + sys.argv[1:], env) else: + # On local development, we prioritize worker packages over + # third-party user packages (in .venv) sys.path.insert(1, func_worker_dir) add_script_root_to_sys_path() from azure_functions_worker import main diff --git a/python/prodV3/worker.py b/python/prodV3/worker.py index d5d78fbb3..69644905d 100644 --- a/python/prodV3/worker.py +++ b/python/prodV3/worker.py @@ -17,6 +17,7 @@ def is_azure_environment(): + '''Check if the function app is running on the cloud''' return (AZURE_CONTAINER_NAME in os.environ or AZURE_WEBSITE_INSTANCE_ID in os.environ) @@ -29,6 +30,16 @@ def add_script_root_to_sys_path(): def determine_user_pkg_paths(): + '''This finds the user packages when function apps are running on the cloud + + For Python 3.6 app, the third-party packages can live in any of the paths: + /home/site/wwwroot/.python_packages/lib/site-packages + /home/site/wwwroot/.python_packages/lib/python3.6/site-packages + /home/site/wwwroot/worker_venv/lib/python3.6/site-packages + + For Python 3.7 and Python 3.8, we only accept: + /home/site/wwwroot/.python_packages/lib/site-packages + ''' minor_version = sys.version_info[1] home = Path.home() @@ -57,12 +68,17 @@ def determine_user_pkg_paths(): user_pkg_paths = determine_user_pkg_paths() joined_pkg_paths = os.pathsep.join(user_pkg_paths) + + # On cloud, we prioritize third-party user packages + # over worker packages in PYTHONPATH env['PYTHONPATH'] = f'{joined_pkg_paths}:{func_worker_dir}' os.execve(sys.executable, [sys.executable, '-m', 'azure_functions_worker'] + sys.argv[1:], env) else: + # On local development, we prioritize worker packages over + # third-party user packages (in .venv) sys.path.insert(1, func_worker_dir) add_script_root_to_sys_path() from azure_functions_worker import main From 06f83749608f592ce06ee7c2be0ddbafaa770167 Mon Sep 17 00:00:00 2001 From: "Hanzhang Zeng (Roger)" Date: Tue, 21 Jul 2020 18:46:56 -0700 Subject: [PATCH 3/3] Added unit tests --- python/test/worker.py | 15 +++++++ .../absolute_thirdparty/function.json | 16 +++++++ .../absolute_thirdparty/main.py | 9 ++++ .../brokenimplicit/function.json | 16 ------- .../implicit_import/function.json | 16 +++++++ .../main.py | 6 ++- .../module_not_found/function.json | 16 +++++++ .../load_functions/module_not_found/main.py | 8 ++++ .../name_collision/function.json | 16 +++++++ .../load_functions/name_collision/main.py | 10 +++++ .../name_collision_app_import/function.json | 16 +++++++ .../name_collision_app_import/main.py | 10 +++++ .../load_functions/pytest/__init__.py | 7 +++ tests/unittests/test_loader.py | 43 +++++++++++++++++-- 14 files changed, 182 insertions(+), 22 deletions(-) create mode 100644 tests/unittests/load_functions/absolute_thirdparty/function.json create mode 100644 tests/unittests/load_functions/absolute_thirdparty/main.py delete mode 100644 tests/unittests/load_functions/brokenimplicit/function.json create mode 100644 tests/unittests/load_functions/implicit_import/function.json rename tests/unittests/load_functions/{brokenimplicit => implicit_import}/main.py (51%) create mode 100644 tests/unittests/load_functions/module_not_found/function.json create mode 100644 tests/unittests/load_functions/module_not_found/main.py create mode 100644 tests/unittests/load_functions/name_collision/function.json create mode 100644 tests/unittests/load_functions/name_collision/main.py create mode 100644 tests/unittests/load_functions/name_collision_app_import/function.json create mode 100644 tests/unittests/load_functions/name_collision_app_import/main.py create mode 100644 tests/unittests/load_functions/pytest/__init__.py diff --git a/python/test/worker.py b/python/test/worker.py index 097c48a0d..e2ef12d22 100644 --- a/python/test/worker.py +++ b/python/test/worker.py @@ -1,4 +1,19 @@ +import sys +import os from azure_functions_worker import main + +# Azure environment variables +AZURE_WEBJOBS_SCRIPT_ROOT = "AzureWebJobsScriptRoot" + + +def add_script_root_to_sys_path(): + '''Append function project root to module finding sys.path''' + functions_script_root = os.getenv(AZURE_WEBJOBS_SCRIPT_ROOT) + if functions_script_root is not None: + sys.path.append(functions_script_root) + + if __name__ == '__main__': + add_script_root_to_sys_path() main.main() diff --git a/tests/unittests/load_functions/absolute_thirdparty/function.json b/tests/unittests/load_functions/absolute_thirdparty/function.json new file mode 100644 index 000000000..cb4469e61 --- /dev/null +++ b/tests/unittests/load_functions/absolute_thirdparty/function.json @@ -0,0 +1,16 @@ +{ + "scriptFile": "main.py", + "entryPoint": "main", + "bindings": [ + { + "type": "httpTrigger", + "direction": "in", + "name": "req" + }, + { + "type": "http", + "direction": "out", + "name": "$return" + } + ] +} diff --git a/tests/unittests/load_functions/absolute_thirdparty/main.py b/tests/unittests/load_functions/absolute_thirdparty/main.py new file mode 100644 index 000000000..e00bf2f3f --- /dev/null +++ b/tests/unittests/load_functions/absolute_thirdparty/main.py @@ -0,0 +1,9 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +# Import a module from thirdparty package azure-eventhub +import azure.eventhub as eh + + +def main(req) -> str: + return f'eh = {eh.__name__}' diff --git a/tests/unittests/load_functions/brokenimplicit/function.json b/tests/unittests/load_functions/brokenimplicit/function.json deleted file mode 100644 index 0c705042c..000000000 --- a/tests/unittests/load_functions/brokenimplicit/function.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "scriptFile": "main.py", - "entryPoint": "customentry", - "bindings": [ - { - "type": "httpTrigger", - "direction": "in", - "name": "req" - }, - { - "type": "http", - "direction": "out", - "name": "$return" - } - ] - } diff --git a/tests/unittests/load_functions/implicit_import/function.json b/tests/unittests/load_functions/implicit_import/function.json new file mode 100644 index 000000000..ec10c6de0 --- /dev/null +++ b/tests/unittests/load_functions/implicit_import/function.json @@ -0,0 +1,16 @@ +{ + "scriptFile": "main.py", + "entryPoint": "implicitinmport", + "bindings": [ + { + "type": "httpTrigger", + "direction": "in", + "name": "req" + }, + { + "type": "http", + "direction": "out", + "name": "$return" + } + ] +} diff --git a/tests/unittests/load_functions/brokenimplicit/main.py b/tests/unittests/load_functions/implicit_import/main.py similarity index 51% rename from tests/unittests/load_functions/brokenimplicit/main.py rename to tests/unittests/load_functions/implicit_import/main.py index 372340d07..96b929ab9 100644 --- a/tests/unittests/load_functions/brokenimplicit/main.py +++ b/tests/unittests/load_functions/implicit_import/main.py @@ -1,8 +1,10 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. -# Import simple module with implicit directory import statement should fail + +# Import simple module with implicit statement should now be acceptable +# since sys.path is now appended with function script root from simple.main import main as s_main -def brokenimplicit(req) -> str: +def implicitinmport(req) -> str: return f's_main = {s_main(req)}' diff --git a/tests/unittests/load_functions/module_not_found/function.json b/tests/unittests/load_functions/module_not_found/function.json new file mode 100644 index 000000000..77ad6ecb1 --- /dev/null +++ b/tests/unittests/load_functions/module_not_found/function.json @@ -0,0 +1,16 @@ +{ + "scriptFile": "main.py", + "entryPoint": "modulenotfound", + "bindings": [ + { + "type": "httpTrigger", + "direction": "in", + "name": "req" + }, + { + "type": "http", + "direction": "out", + "name": "$return" + } + ] +} diff --git a/tests/unittests/load_functions/module_not_found/main.py b/tests/unittests/load_functions/module_not_found/main.py new file mode 100644 index 000000000..82e43227b --- /dev/null +++ b/tests/unittests/load_functions/module_not_found/main.py @@ -0,0 +1,8 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +# Import simple module with implicit statement should now be acceptable +import notfound + + +def modulenotfound(req) -> str: + return f'notfound = {notfound.__name__}' diff --git a/tests/unittests/load_functions/name_collision/function.json b/tests/unittests/load_functions/name_collision/function.json new file mode 100644 index 000000000..cb4469e61 --- /dev/null +++ b/tests/unittests/load_functions/name_collision/function.json @@ -0,0 +1,16 @@ +{ + "scriptFile": "main.py", + "entryPoint": "main", + "bindings": [ + { + "type": "httpTrigger", + "direction": "in", + "name": "req" + }, + { + "type": "http", + "direction": "out", + "name": "$return" + } + ] +} diff --git a/tests/unittests/load_functions/name_collision/main.py b/tests/unittests/load_functions/name_collision/main.py new file mode 100644 index 000000000..41924a4c7 --- /dev/null +++ b/tests/unittests/load_functions/name_collision/main.py @@ -0,0 +1,10 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +# Both customer code and third-party package has the same name pytest. +# Worker should pick the pytest from the third-party package +import pytest as pt + + +def main(req) -> str: + return f'pt.__version__ = {pt.__version__}' diff --git a/tests/unittests/load_functions/name_collision_app_import/function.json b/tests/unittests/load_functions/name_collision_app_import/function.json new file mode 100644 index 000000000..cb4469e61 --- /dev/null +++ b/tests/unittests/load_functions/name_collision_app_import/function.json @@ -0,0 +1,16 @@ +{ + "scriptFile": "main.py", + "entryPoint": "main", + "bindings": [ + { + "type": "httpTrigger", + "direction": "in", + "name": "req" + }, + { + "type": "http", + "direction": "out", + "name": "$return" + } + ] +} diff --git a/tests/unittests/load_functions/name_collision_app_import/main.py b/tests/unittests/load_functions/name_collision_app_import/main.py new file mode 100644 index 000000000..4767a1c7d --- /dev/null +++ b/tests/unittests/load_functions/name_collision_app_import/main.py @@ -0,0 +1,10 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +# Both customer code and third-party package has the same name pytest. +# When using absolute import, should pick customer's package. +import __app__.pytest as pt + + +def main(req) -> str: + return f'pt.__version__ = {pt.__version__}' diff --git a/tests/unittests/load_functions/pytest/__init__.py b/tests/unittests/load_functions/pytest/__init__.py new file mode 100644 index 000000000..4349b0878 --- /dev/null +++ b/tests/unittests/load_functions/pytest/__init__.py @@ -0,0 +1,7 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + + +'''This module pytest is provided inside customer's code, +used for checking module name collision''' +__version__ = 'from.customer.code' diff --git a/tests/unittests/test_loader.py b/tests/unittests/test_loader.py index 482996ef9..7ab9c041b 100644 --- a/tests/unittests/test_loader.py +++ b/tests/unittests/test_loader.py @@ -45,13 +45,48 @@ def test_loader_parentmodule(self): self.assertEqual(r.status_code, 200) self.assertEqual(r.text, '__app__.parentmodule.module') - def test_loader_brokenimplicit(self): - r = self.webhost.request('GET', 'brokenimplicit') + def test_loader_absolute_thirdparty(self): + ''' Allow third-party package import from .python_packages + and worker_venv''' + + r = self.webhost.request('GET', 'absolute_thirdparty') + self.assertEqual(r.status_code, 200) + self.assertEqual(r.text, 'eh = azure.eventhub') + + def test_loader_prioritize_customer_module(self): + ''' When a module in customer code has the same name with a third-party + package, the worker should prioritize third-party package''' + + r = self.webhost.request('GET', 'name_collision') + self.assertEqual(r.status_code, 200) + self.assertRegex(r.text, r'pt.__version__ = \d+.\d+.\d+') + + def test_loader_fix_customer_module_with_app_import(self): + ''' When a module in customer code has the same name with a third-party + package, if customer uses "import __app__." statement, + the worker should load customer package''' + + r = self.webhost.request('GET', 'name_collision_app_import') + self.assertEqual(r.status_code, 200) + self.assertEqual(r.text, 'pt.__version__ = from.customer.code') + + def test_loader_implicit_import(self): + ''' Since sys.path is now fixed with script root appended, + implicit import statement is now acceptable.''' + + r = self.webhost.request('GET', 'implicit_import') + self.assertEqual(r.status_code, 200) + self.assertEqual(r.text, 's_main = simple.main') + + def test_loader_module_not_found(self): + ''' If a module cannot be found, should throw an exception with + trouble shooting link https://aka.ms/functions-modulenotfound''' + r = self.webhost.request('GET', 'module_not_found') self.assertEqual(r.status_code, 500) - def check_log_loader_brokenimplicit(self, host_out): + def check_log_loader_module_not_found(self, host_out): self.assertIn("Exception: ModuleNotFoundError: " - "No module named 'simple'. " + "No module named 'notfound'. " "Troubleshooting Guide: " "https://aka.ms/functions-modulenotfound", host_out)