Yicong-Huang commented on code in PR #5046:
URL: https://github.com/apache/texera/pull/5046#discussion_r3232557945


##########
amber/src/main/python/core/architecture/managers/executor_manager.py:
##########
@@ -114,10 +114,24 @@ def load_executor_definition(self, code: str) -> 
type(Operator):
     def close(self) -> None:
         """
         Close the tmp fs and release all resources created within it.
+        This also evicts the loaded operator module from ``sys.modules``
+        and removes the tmp fs path from ``sys.path`` so a single call
+        fully reverses every global side-effect performed by ``fs`` and
+        ``load_executor_definition``.
         :return:
         """
+        if "fs" not in self.__dict__:
+            # fs was never materialized; nothing to clean up.
+            return
+        root = self.fs.getsyspath("/")
         self.fs.close()
-        logger.debug(f"Tmp directory {self.fs.getsyspath('/')} is closed and 
cleared.")
+        try:
+            sys.path.remove(str(Path(root)))
+        except ValueError:
+            pass

Review Comment:
   I believe tempfs can do the clean up properly by itself. thus we don't need 
to manually run remove command, which is also very dangerous. 
   
   I suggest you find our reference to `self._fs` or `self.fs` are they 
duplicating, or being cached anywhere, causing the fs not being properly 
destructed in `self.fs.close()`.  ideally the code here should simply just 
`self.fs.close()`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to