diff --git a/src/query/procedure/module.cpp b/src/query/procedure/module.cpp index f169bcc11..a063881e4 100644 --- a/src/query/procedure/module.cpp +++ b/src/query/procedure/module.cpp @@ -21,7 +21,6 @@ extern "C" { #include #include -#include #include "py/py.hpp" #include "query/procedure/mg_procedure_helpers.hpp" #include "query/procedure/py_module.hpp" @@ -53,10 +52,6 @@ constexpr const char *func_code = "node_iter.visit_ImportFrom = visit_ImportFrom\n" "node_iter.visit(ast.parse(code))\n"; -// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)w -DEFINE_string(python_submodules_directory, "mage", - "Directory in which the Python submodules' utility procedures are saved."); - void ProcessFileDependencies(std::filesystem::path file_path_, const char *module_path, const char *func_code, PyObject *sys_mod_ref); @@ -1027,37 +1022,33 @@ bool PythonModule::Close() { py::Object sys(PyImport_ImportModule("sys")); PyObject *sys_mod_ref = sys.GetAttr("modules").Ptr(); - std::filesystem::path submodules_path = file_path_.parent_path(); - std::string_view stem = std::string_view(file_path_.stem().c_str()); - submodules_path /= FLAGS_python_submodules_directory; + std::string stem = file_path_.stem().string(); ProcessFileDependencies(file_path_, file_path_.stem().c_str(), func_code, sys_mod_ref); - if (std::filesystem::exists(submodules_path)) { - std::filesystem::path submodules; + std::vector submodules; - for (auto const &dir_entry : std::filesystem::directory_iterator(submodules_path)) { - std::string_view dir_entry_stem = std::string_view(dir_entry.path().stem().c_str()); - if (dir_entry.is_regular_file() || dir_entry_stem.compare("__pycache__") == 0) continue; - if (dir_entry_stem.find(stem) != std::string_view::npos && - (submodules.empty() || std::string_view(submodules.stem().c_str()).length() > dir_entry_stem.length())) { - submodules = dir_entry.path(); - } + for (auto it = std::filesystem::recursive_directory_iterator(file_path_.parent_path()); + it != std::filesystem::recursive_directory_iterator(); ++it) { + std::string dir_entry_stem = it->path().stem().string(); + if (it->is_regular_file() || dir_entry_stem == "__pycache__") continue; + if (dir_entry_stem.find(stem) != std::string_view::npos) { + it.disable_recursion_pending(); + submodules.emplace_back(it->path()); } + } - if (std::filesystem::exists(submodules)) { - if (!std::filesystem::remove_all(submodules / "__pycache__")) { - spdlog::trace("Submodules cache couldn't be cleared!"); - } else { - for (auto const &rec_dir_entry : std::filesystem::recursive_directory_iterator(submodules)) { - std::string_view rec_dir_entry_stem = std::string_view(rec_dir_entry.path().stem().c_str()); - if (rec_dir_entry.is_directory() && rec_dir_entry_stem.compare("__pycache__") != 0) { - std::filesystem::remove_all(rec_dir_entry.path() / "__pycache__"); - } - std::string_view rec_dir_entry_ext = std::string_view(rec_dir_entry.path().extension().c_str()); - if (!rec_dir_entry.is_regular_file() || rec_dir_entry_ext.compare(".py") != 0) continue; - ProcessFileDependencies(rec_dir_entry.path().c_str(), file_path_.stem().c_str(), func_code, sys_mod_ref); + for (const auto &submodule : submodules) { + if (std::filesystem::exists(submodule)) { + std::filesystem::remove_all(submodule / "__pycache__"); + for (auto const &rec_dir_entry : std::filesystem::recursive_directory_iterator(submodule)) { + std::string rec_dir_entry_stem = rec_dir_entry.path().stem().string(); + if (rec_dir_entry.is_directory() && rec_dir_entry_stem != "__pycache__") { + std::filesystem::remove_all(rec_dir_entry.path() / "__pycache__"); } + std::string rec_dir_entry_ext = rec_dir_entry.path().extension().string(); + if (!rec_dir_entry.is_regular_file() || rec_dir_entry_ext != ".py") continue; + ProcessFileDependencies(rec_dir_entry.path().c_str(), file_path_.stem().c_str(), func_code, sys_mod_ref); } } } @@ -1097,7 +1088,7 @@ void ProcessFileDependencies(std::filesystem::path file_path_, const char *modul if (iterator != nullptr) { while ((module = PyIter_Next(iterator))) { const char *module_name = PyUnicode_AsUTF8(module); - auto module_name_str = std::string_view(module_name); + auto module_name_str = std::string(module_name); PyObject *sys_iterator = PyObject_GetIter(PyDict_Keys(sys_mod_ref)); if (sys_iterator == nullptr) { spdlog::warn("Cannot get reference to the sys.modules.keys()"); @@ -1106,7 +1097,7 @@ void ProcessFileDependencies(std::filesystem::path file_path_, const char *modul PyObject *sys_mod_key = nullptr; while ((sys_mod_key = PyIter_Next(sys_iterator))) { const char *sys_mod_key_name = PyUnicode_AsUTF8(sys_mod_key); - auto sys_mod_key_name_str = std::string_view(sys_mod_key_name); + auto sys_mod_key_name_str = std::string(sys_mod_key_name); if (sys_mod_key_name_str.rfind(module_name_str, 0) == 0 && sys_mod_key_name_str.compare(module_path) != 0) { PyDict_DelItemString(sys_mod_ref, sys_mod_key_name); // don't test output } diff --git a/tests/e2e/configuration/default_config.py b/tests/e2e/configuration/default_config.py index 6ee26a002..a60d1b258 100644 --- a/tests/e2e/configuration/default_config.py +++ b/tests/e2e/configuration/default_config.py @@ -169,9 +169,4 @@ startup_config_dict = { "Path to cypherl file that is used for configuring users and database schema before server starts.", ), "init_data_file": ("", "", "Path to cypherl file that is used for creating data after server starts."), - "python_submodules_directory": ( - "mage", - "mage", - "Directory in which the Python submodules' utility procedures are saved.", - ), } diff --git a/tests/e2e/python_query_modules_reloading/procedures/CMakeLists.txt b/tests/e2e/python_query_modules_reloading/procedures/CMakeLists.txt index 1ab572fba..703d4e8c5 100644 --- a/tests/e2e/python_query_modules_reloading/procedures/CMakeLists.txt +++ b/tests/e2e/python_query_modules_reloading/procedures/CMakeLists.txt @@ -1,3 +1,6 @@ copy_query_modules_reloading_procedures_e2e_python_files(test_module.py) +copy_query_modules_reloading_procedures_e2e_python_files(new_test_module.py) add_subdirectory(mage) +add_subdirectory(new_test_module_utils) + diff --git a/tests/e2e/python_query_modules_reloading/procedures/new_test_module.py b/tests/e2e/python_query_modules_reloading/procedures/new_test_module.py new file mode 100644 index 000000000..36366a99a --- /dev/null +++ b/tests/e2e/python_query_modules_reloading/procedures/new_test_module.py @@ -0,0 +1,14 @@ +import mgp + +# isort: off +# fmt: off +from new_test_module_utils.new_test_functions import \ + test_function as test_function1 +from new_test_module_utils.new_test_functions_dir.new_test_subfunctions import \ + test_subfunction as test_function2 +# fmt: on + + +@mgp.read_proc +def test(ctx: mgp.ProcCtx, a: mgp.Number, b: mgp.Number) -> mgp.Record(result1=mgp.Number, result2=mgp.Number): + return mgp.Record(result1=test_function1(a, b), result2=test_function2(a, b)) diff --git a/tests/e2e/python_query_modules_reloading/procedures/new_test_module_utils/CMakeLists.txt b/tests/e2e/python_query_modules_reloading/procedures/new_test_module_utils/CMakeLists.txt new file mode 100644 index 000000000..22af6ab14 --- /dev/null +++ b/tests/e2e/python_query_modules_reloading/procedures/new_test_module_utils/CMakeLists.txt @@ -0,0 +1,3 @@ +copy_query_modules_reloading_procedures_e2e_python_files(new_test_functions.py) + +add_subdirectory(new_test_functions_dir) diff --git a/tests/e2e/python_query_modules_reloading/procedures/new_test_module_utils/new_test_functions.py b/tests/e2e/python_query_modules_reloading/procedures/new_test_module_utils/new_test_functions.py new file mode 100644 index 000000000..d6932e63f --- /dev/null +++ b/tests/e2e/python_query_modules_reloading/procedures/new_test_module_utils/new_test_functions.py @@ -0,0 +1,2 @@ +def test_function(a: int, b: int) -> int: + return a + b diff --git a/tests/e2e/python_query_modules_reloading/procedures/new_test_module_utils/new_test_functions_dir/CMakeLists.txt b/tests/e2e/python_query_modules_reloading/procedures/new_test_module_utils/new_test_functions_dir/CMakeLists.txt new file mode 100644 index 000000000..a863f5c39 --- /dev/null +++ b/tests/e2e/python_query_modules_reloading/procedures/new_test_module_utils/new_test_functions_dir/CMakeLists.txt @@ -0,0 +1 @@ +copy_query_modules_reloading_procedures_e2e_python_files(new_test_subfunctions.py) diff --git a/tests/e2e/python_query_modules_reloading/procedures/new_test_module_utils/new_test_functions_dir/new_test_subfunctions.py b/tests/e2e/python_query_modules_reloading/procedures/new_test_module_utils/new_test_functions_dir/new_test_subfunctions.py new file mode 100644 index 000000000..24d9be09a --- /dev/null +++ b/tests/e2e/python_query_modules_reloading/procedures/new_test_module_utils/new_test_functions_dir/new_test_subfunctions.py @@ -0,0 +1,2 @@ +def test_subfunction(a: int, b: int) -> int: + return a * b diff --git a/tests/e2e/python_query_modules_reloading/test_reload_query_module.py b/tests/e2e/python_query_modules_reloading/test_reload_query_module.py index c2fef4564..7f7205d82 100644 --- a/tests/e2e/python_query_modules_reloading/test_reload_query_module.py +++ b/tests/e2e/python_query_modules_reloading/test_reload_query_module.py @@ -9,35 +9,50 @@ # by the Apache License, Version 2.0, included in the file # licenses/APL.txt. + import os # To be removed import sys import pytest from common import connect, execute_and_fetch_all -COMMON_PATH_PREFIX = "procedures/mage/test_module" +COMMON_PATH_PREFIX_TEST1 = "procedures/mage/test_module" +COMMON_PATH_PREFIX_TEST2 = "procedures/new_test_module_utils" + FUNC1_PATH = os.path.join( os.path.dirname(__file__), - COMMON_PATH_PREFIX, + COMMON_PATH_PREFIX_TEST1, "test_functions.py", ) FUNC2_PATH = os.path.join( os.path.dirname(__file__), - COMMON_PATH_PREFIX, + COMMON_PATH_PREFIX_TEST1, "test_functions_dir/test_subfunctions.py", ) +FUNC3_PATH = os.path.join( + os.path.dirname(__file__), + COMMON_PATH_PREFIX_TEST2, + "new_test_functions.py", +) -def preprocess_functions(): - with open(FUNC1_PATH, "w") as func1_file: +FUNC4_PATH = os.path.join( + os.path.dirname(__file__), + COMMON_PATH_PREFIX_TEST2, + "new_test_functions_dir/new_test_subfunctions.py", +) + + +def preprocess_functions(path1: str, path2: str): + with open(path1, "w") as func1_file: func1_file.write( """def test_function(a: int, b: int) -> int: return a - b """ ) - with open(FUNC2_PATH, "w") as func2_file: + with open(path2, "w") as func2_file: func2_file.write( """def test_subfunction(a: int, b: int) -> int: return a / b @@ -45,15 +60,15 @@ def preprocess_functions(): ) -def postprocess_functions(): - with open(FUNC1_PATH, "w") as func1_file: +def postprocess_functions(path1: str, path2: str): + with open(path1, "w") as func1_file: func1_file.write( """def test_function(a: int, b: int) -> int: return a + b """ ) - with open(FUNC2_PATH, "w") as func2_file: + with open(path2, "w") as func2_file: func2_file.write( """def test_subfunction(a: int, b: int) -> int: return a * b @@ -61,6 +76,56 @@ def postprocess_functions(): ) +def test_mg_load_reload_submodule_root_utils(): + """Tests whether mg.load reloads content of some submodule code.""" + cursor = connect().cursor() + # First do a simple experiment + test_module_res = execute_and_fetch_all(cursor, "CALL new_test_module.test(10, 2) YIELD * RETURN *;") + try: + assert test_module_res[0][0] == 12 # + operator + assert test_module_res[0][1] == 20 # * operator + # Now modify content of test function + preprocess_functions(FUNC3_PATH, FUNC4_PATH) + # Test that it doesn't work without calling reload + test_module_res = execute_and_fetch_all(cursor, "CALL new_test_module.test(10, 2) YIELD * RETURN *;") + assert test_module_res[0][0] == 12 # + operator + assert test_module_res[0][1] == 20 # * operator + # Reload module + execute_and_fetch_all(cursor, "CALL mg.load('new_test_module');") + test_module_res = execute_and_fetch_all(cursor, "CALL new_test_module.test(10, 2) YIELD * RETURN *;") + assert test_module_res[0][0] == 8 # - operator + assert test_module_res[0][1] == 5 # / operator + finally: + # Revert to the original state for the consistency + postprocess_functions(FUNC3_PATH, FUNC4_PATH) + execute_and_fetch_all(cursor, "CALL mg.load('new_test_module');") + + +def test_mg_load_all_reload_submodule_root_utils(): + """Tests whether mg.load_all reloads content of some submodule code""" + cursor = connect().cursor() + # First do a simple experiment + test_module_res = execute_and_fetch_all(cursor, "CALL new_test_module.test(10, 2) YIELD * RETURN *;") + try: + assert test_module_res[0][0] == 12 # + operator + assert test_module_res[0][1] == 20 # * operator + # Now modify content of test function + preprocess_functions(FUNC3_PATH, FUNC4_PATH) + # Test that it doesn't work without calling reload + test_module_res = execute_and_fetch_all(cursor, "CALL new_test_module.test(10, 2) YIELD * RETURN *;") + assert test_module_res[0][0] == 12 # + operator + assert test_module_res[0][1] == 20 # * operator + # Reload module + execute_and_fetch_all(cursor, "CALL mg.load_all();") + test_module_res = execute_and_fetch_all(cursor, "CALL new_test_module.test(10, 2) YIELD * RETURN *;") + assert test_module_res[0][0] == 8 # - operator + assert test_module_res[0][1] == 5 # / operator + finally: + # Revert to the original state for the consistency + postprocess_functions(FUNC3_PATH, FUNC4_PATH) + execute_and_fetch_all(cursor, "CALL mg.load_all();") + + def test_mg_load_reload_submodule(): """Tests whether mg.load reloads content of some submodule code.""" cursor = connect().cursor() @@ -70,7 +135,7 @@ def test_mg_load_reload_submodule(): assert test_module_res[0][0] == 12 # + operator assert test_module_res[0][1] == 20 # * operator # Now modify content of test function - preprocess_functions() + preprocess_functions(FUNC1_PATH, FUNC2_PATH) # Test that it doesn't work without calling reload test_module_res = execute_and_fetch_all(cursor, "CALL test_module.test(10, 2) YIELD * RETURN *;") assert test_module_res[0][0] == 12 # + operator @@ -82,7 +147,7 @@ def test_mg_load_reload_submodule(): assert test_module_res[0][1] == 5 # / operator finally: # Revert to the original state for the consistency - postprocess_functions() + postprocess_functions(FUNC1_PATH, FUNC2_PATH) execute_and_fetch_all(cursor, "CALL mg.load('test_module');") @@ -95,7 +160,7 @@ def test_mg_load_all_reload_submodule(): assert test_module_res[0][0] == 12 # + operator assert test_module_res[0][1] == 20 # * operator # Now modify content of test function - preprocess_functions() + preprocess_functions(FUNC1_PATH, FUNC2_PATH) # Test that it doesn't work without calling reload test_module_res = execute_and_fetch_all(cursor, "CALL test_module.test(10, 2) YIELD * RETURN *;") assert test_module_res[0][0] == 12 # + operator @@ -107,7 +172,7 @@ def test_mg_load_all_reload_submodule(): assert test_module_res[0][1] == 5 # / operator finally: # Revert to the original state for the consistency - postprocess_functions() + postprocess_functions(FUNC1_PATH, FUNC2_PATH) execute_and_fetch_all(cursor, "CALL mg.load_all();")