Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

code loading errors are not reported on resource action log #8722

Closed
sanderr opened this issue Feb 10, 2025 · 1 comment
Closed

code loading errors are not reported on resource action log #8722

sanderr opened this issue Feb 10, 2025 · 1 comment
Assignees
Labels
bug Something isn't working usability This ticket is related to improved usability

Comments

@sanderr
Copy link
Contributor

sanderr commented Feb 10, 2025

load_module in loader.py catches ImportError and logs a warning. As a result, its caller does not see the exception at all. The caller is the one that registers exceptions on the executor instance so that they can be reported to the resource action logs.

This was discovered when investigating a code loading issue reported on Slack.

I believe the following diff should fix the issue, but it needs to be verified, a test is required, and cleanup may be needed.

diff --git a/src/inmanta/agent/forking_executor.py b/src/inmanta/agent/forking_executor.py
index f629eddc8..39f9ae9de 100644
--- a/src/inmanta/agent/forking_executor.py
+++ b/src/inmanta/agent/forking_executor.py
@@ -446,7 +446,7 @@ class InitCommand(inmanta.protocol.ipc_light.IPCMethod[ExecutorContext, typing.S
                     functools.partial(loader.load_module, module_source.name, module_source.hash_value),
                 )
             except Exception as e:
-                logger.info("Failed to load sources: %s", module_source, exc_info=True)
+                logger.info("Failed to load module: %s", module_source, exc_info=True)
                 failed.append(
                     inmanta.loader.FailedModuleSource(
                         module_source=module_source,
diff --git a/src/inmanta/loader.py b/src/inmanta/loader.py
index bd34c8ae4..8643e2bc9 100644
--- a/src/inmanta/loader.py
+++ b/src/inmanta/loader.py
@@ -288,7 +288,7 @@ class CodeLoader:

     def load_module(self, mod_name: str, hv: str) -> None:
         """
-        Ensure the given module is loaded.
+        Ensure the given module is loaded. Does not capture any import errors.

         :param mod_name: Name of the module to load
         :param hv: hash value of the content of the module
@@ -298,18 +298,15 @@ class CodeLoader:

         # Importing a module -> only the first import loads the code
         # cache of loaded modules mechanism -> starts afresh when agent is restarted
-        try:
-            if mod_name in self.__modules:
-                if hv != self.__modules[mod_name][0]:
-                    raise Exception(f"The content of module {mod_name} changed since it was last imported.")
-                LOGGER.debug("Module %s is already loaded", mod_name)
-                return
-            else:
-                mod = importlib.import_module(mod_name)
-            self.__modules[mod_name] = (hv, mod)
-            LOGGER.info("Loaded module %s", mod_name)
-        except ImportError:
-            LOGGER.exception("Unable to load module %s", mod_name)
+        if mod_name in self.__modules:
+            if hv != self.__modules[mod_name][0]:
+                raise Exception(f"The content of module {mod_name} changed since it was last imported.")
+            LOGGER.debug("Module %s is already loaded", mod_name)
+            return
+        else:
+            mod = importlib.import_module(mod_name)
+        self.__modules[mod_name] = (hv, mod)
+        LOGGER.info("Loaded module %s", mod_name)

     def install_source(self, module_source: ModuleSource) -> None:
         """
diff --git a/src/inmanta/util/__init__.py b/src/inmanta/util/__init__.py
index ab8f7d8cd..7fbe6a86c 100644
--- a/src/inmanta/util/__init__.py
+++ b/src/inmanta/util/__init__.py
@@ -845,7 +845,7 @@ def ensure_event_loop() -> asyncio.AbstractEventLoop:
     """
     try:
         # nothing needs to be done if this thread already has an event loop
-        return asyncio.get_event_loop()
+        return asyncio.get_running_loop()
     except RuntimeError:
         # asyncio.set_event_loop sets the event loop for this thread only
         new_loop = asyncio.new_event_loop()
@sanderr sanderr added bug Something isn't working usability This ticket is related to improved usability labels Feb 10, 2025
@wouterdb
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working usability This ticket is related to improved usability
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants