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, if we use it
properly. 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]
