Append /home/site/wwwroot to sys.path (#726)

Hazhzeng · web-flow · commit ff38583164b4 · 2020-08-25T17:31:02.000-07:00
* Add sys.path reload in Linux Consumption and add /home/site/wwwroot in Linux Dedicated/Premium
diff --git 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
@@ -13,14 +13,33 @@
 # Azure environment variables
 AZURE_WEBSITE_INSTANCE_ID = "WEBSITE_INSTANCE_ID"
 AZURE_CONTAINER_NAME = "CONTAINER_NAME"
+AZURE_WEBJOBS_SCRIPT_ROOT = "AzureWebJobsScriptRoot"
 
 
 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)
 
 
+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():
+    '''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()
@@ -49,13 +68,19 @@ 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
 
         main.main()
diff --git a/python/prodV3/worker.py b/python/prodV3/worker.py
@@ -13,14 +13,33 @@
 # Azure environment variables
 AZURE_WEBSITE_INSTANCE_ID = "WEBSITE_INSTANCE_ID"
 AZURE_CONTAINER_NAME = "CONTAINER_NAME"
+AZURE_WEBJOBS_SCRIPT_ROOT = "AzureWebJobsScriptRoot"
 
 
 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)
 
 
+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():
+    '''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()
@@ -49,13 +68,19 @@ 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
 
         main.main()
diff --git 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
@@ -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
@@ -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
diff --git a/tests/unittests/load_functions/implicit_import/function.json 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/implicit_import/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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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__.<module>" 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)
 
