Ma77Ball commented on code in PR #5046:
URL: https://github.com/apache/texera/pull/5046#discussion_r3232716168
##########
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 looked into it as you suggested:
- I didn't find any duplicate/cached self.fs reference. self.fs.close()
does call shutil.rmtree internally to delete the tmp dir on disk.
- sys.path is just Python's in-memory list of import directories, not a file
system call, and .remove() is just the list method, not related to os.remove.
Nothing on disk is touched or removed.
Those two cleanup lines exist to undo (tempfs doesn't know about these):
- Line 71: where we did sys.path.append(str(root)) --> which needs to be
removeed.
- Line 105: we did importlib.import_module(...) --> which needs to be poped
from sys.modules.
Tempfs only owns the temp dir, so it can only clean up the temp dir. The
sys.path / sys.modules entries still need to be cleaned after this.
--
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]
