diff --git a/armi/runLog.py b/armi/runLog.py index 5fd6b17990..c867859ae6 100644 --- a/armi/runLog.py +++ b/armi/runLog.py @@ -60,6 +60,8 @@ self._log({1}, message, args, **kws) logging.Logger.{0} = {0}""" LOG_DIR = os.path.join(os.getcwd(), "logs") +if os.environ.get("TEMP_ROOT_PATH"): + LOG_DIR = os.path.join(os.environ["TEMP_ROOT_PATH"], "logs") OS_SECONDS_TIMEOUT = 2 * 60 SEP = "|" STDERR_LOGGER_NAME = "ARMI_ERROR" diff --git a/armi/tests/test_mpiFeatures.py b/armi/tests/test_mpiFeatures.py index 46fbdc7d2a..1ce96eec25 100644 --- a/armi/tests/test_mpiFeatures.py +++ b/armi/tests/test_mpiFeatures.py @@ -297,50 +297,50 @@ class MpiPathToolsTests(unittest.TestCase): def test_cleanPathMpi(self): """Simple tests of cleanPath(), in the MPI scenario.""" with TemporaryDirectoryChanger(): - # TEST 0: File is not safe to delete, due to name pathing + # TEST 0: File is not safe to delete, due to due not being a temp dir or under FAST_PATH filePath0 = "test0_cleanPathNoMpi" open(filePath0, "w").write("something") - self.assertTrue(os.path.exists(filePath0)) with self.assertRaises(Exception): pathTools.cleanPath(filePath0, mpiRank=context.MPI_RANK) MPI_COMM.barrier() - # TEST 1: Delete a single file - filePath1 = "test1_cleanPathNoMpi_mongoose" + # TEST 1: Delete a single file under FAST_PATH + filePath1 = os.path.join(context.getFastPath(), "test1_cleanPathNoMpi") open(filePath1, "w").write("something") - self.assertTrue(os.path.exists(filePath1)) pathTools.cleanPath(filePath1, mpiRank=context.MPI_RANK) MPI_COMM.barrier() self.assertFalse(os.path.exists(filePath1)) - # TEST 2: Delete an empty directory - dir2 = "mongoose" + # TEST 2: Delete an empty directory under FAST_PATH + dir2 = os.path.join(context.getFastPath(), "gimmeonereason") os.mkdir(dir2) - self.assertTrue(os.path.exists(dir2)) pathTools.cleanPath(dir2, mpiRank=context.MPI_RANK) MPI_COMM.barrier() self.assertFalse(os.path.exists(dir2)) - # TEST 3: Delete a directory with two files inside - # create directory - dir3 = "mongoose" + # TEST 3: Delete an empty directory with tempDir=True + dir3 = "tostayhere" os.mkdir(dir3) - - # throw in a couple of simple text files - open(os.path.join(dir3, "file1.txt"), "w").write("something1") - open(os.path.join(dir3, "file2.txt"), "w").write("something2") - - # delete the directory and test self.assertTrue(os.path.exists(dir3)) - self.assertTrue(os.path.exists(os.path.join(dir3, "file1.txt"))) - self.assertTrue(os.path.exists(os.path.join(dir3, "file2.txt"))) - pathTools.cleanPath(dir3, mpiRank=context.MPI_RANK) + pathTools.cleanPath(dir3, mpiRank=context.MPI_RANK, tempDir=True) MPI_COMM.barrier() self.assertFalse(os.path.exists(dir3)) + # TEST 3: Delete a directory with two files inside with tempDir=True + dir4 = "andilldirrightbackaround" + os.mkdir(dir4) + open(os.path.join(dir4, "file1.txt"), "w").write("something1") + open(os.path.join(dir4, "file2.txt"), "w").write("something2") + self.assertTrue(os.path.exists(dir4)) + self.assertTrue(os.path.exists(os.path.join(dir4, "file1.txt"))) + self.assertTrue(os.path.exists(os.path.join(dir4, "file2.txt"))) + pathTools.cleanPath(dir4, mpiRank=context.MPI_RANK, tempDir=True) + MPI_COMM.barrier() + self.assertFalse(os.path.exists(dir4)) + class TestContextMpi(unittest.TestCase): """Parallel tests for the Context module.""" diff --git a/armi/utils/directoryChangers.py b/armi/utils/directoryChangers.py index d4f48dbed0..c1e8f9b49b 100644 --- a/armi/utils/directoryChangers.py +++ b/armi/utils/directoryChangers.py @@ -252,6 +252,11 @@ def __init__( outputPath, ) + # If an application sets this environment variable, all root args in all `TempDirChanger` uses are overriden + # with a different root path. This is useful for running unit tests in a read-only environment. + if os.environ.get("TEMP_ROOT_PATH"): + root = os.environ["TEMP_ROOT_PATH"] + # If no root dir is given, the default path comes from context.getFastPath, which # *might* be relative to the cwd, making it possible to delete unintended files. # So this check is here to ensure that if we grab a path from context, it is a @@ -293,7 +298,7 @@ def __enter__(self): def __exit__(self, exc_type, exc_value, traceback): DirectoryChanger.__exit__(self, exc_type, exc_value, traceback) try: - pathTools.cleanPath(self.destination, context.MPI_RANK) + pathTools.cleanPath(self.destination, mpiRank=context.MPI_RANK, tempDir=True) except PermissionError: if os.name == "nt": runLog.warning( diff --git a/armi/utils/outputCache.py b/armi/utils/outputCache.py index 389a47ee41..a0258cbad1 100644 --- a/armi/utils/outputCache.py +++ b/armi/utils/outputCache.py @@ -176,7 +176,8 @@ def deleteCache(cachedFolder): if "cache" not in str(cachedFolder).lower(): raise RuntimeError("Cache location must contain keyword: `cache`.") - cleanPath(cachedFolder) + # We can consider caches temporary directories, since they are not important containers for simulation results. + cleanPath(cachedFolder, tempDir=True) def cacheCall(cacheDir, executablePath, inputPaths, outputFileNames, execute=None, tearDown=None): diff --git a/armi/utils/pathTools.py b/armi/utils/pathTools.py index 7ffd767df0..3b4c844e20 100644 --- a/armi/utils/pathTools.py +++ b/armi/utils/pathTools.py @@ -26,16 +26,6 @@ from armi import context, runLog from armi.utils import safeCopy -DO_NOT_CLEAN_PATHS = [ - "armiruns", - "failedruns", - "mc2run", - "mongoose", - "shufflebranches", - "snapshot", - "tests", -] - def armiAbsPath(*pathParts): """Convert a list of path components to an absolute path, without drive letters if possible.""" @@ -186,19 +176,12 @@ def moduleAndAttributeExist(pathAttr): return moduleAttributeName in userSpecifiedModule.__dict__ -def cleanPath(path, mpiRank=0): - """Recursively delete a path. - - !!! Be careful with this !!! It can delete the entire cluster. - - We add copious os.path.exists checks in case an MPI set of things is trying to delete everything - at the same time. Always check filenames for some special flag when calling this, especially - with full permissions on the cluster. You could accidentally delete everyone's work with one - misplaced line! This doesn't ask questions. +def cleanPath(path, mpiRank=0, tempDir=False): + """Recursively delete a path. This function checks for a few cases we know to be OK to delete: (1) Any + `TemporaryDirectoryChanger` instance and (2) anything under the ARMI `_FAST_PATH`. - Safety nets include an allow-list of paths. - - This makes use of shutil.rmtree and os.remove + Be careful with editing this! Do not make it a generic can-delete-anything function, because it could in theory + delete anything a user has write permissions on. Returns ------- @@ -209,17 +192,19 @@ def cleanPath(path, mpiRank=0): if not os.path.exists(path): return True - for validPath in DO_NOT_CLEAN_PATHS: - if validPath in path.lower(): - valid = True + # Any tempDir can be deleted + if tempDir: + valid = True - if pathlib.Path(context.APP_DATA) in pathlib.Path(path).parents: + # If the path slated for deletion is a subdirectory of _FAST_PATH, then cool, delete. + # _FAST_PATH itself gets deleted on program exit. + if pathlib.Path(path).is_relative_to(pathlib.Path(context.getFastPath())): valid = True if not valid: - raise Exception("You tried to delete {0}, but it does not seem safe to do so.".format(path)) + raise Exception(f"You tried to delete {path}, but it does not seem safe to do so.") - # delete the file/directory from only one process + # Delete the file/directory from only one process if mpiRank == context.MPI_RANK: if os.path.exists(path) and os.path.isdir(path): shutil.rmtree(path) @@ -227,7 +212,7 @@ def cleanPath(path, mpiRank=0): # it's just a file. Delete it. os.remove(path) - # Potentially, wait for the deletion to finish. + # Deletions are not immediate, so wait for it to finish. maxLoops = 6 waitTime = 0.5 loopCounter = 0 diff --git a/armi/utils/tests/test_pathTools.py b/armi/utils/tests/test_pathTools.py index ed258487f3..f15f5eeb7b 100644 --- a/armi/utils/tests/test_pathTools.py +++ b/armi/utils/tests/test_pathTools.py @@ -104,46 +104,46 @@ def test_moduleAndAttributeExist(self): def test_cleanPathNoMpi(self): """Simple tests of cleanPath(), in the no-MPI scenario.""" with TemporaryDirectoryChanger(): - # TEST 0: File is not safe to delete, due to name pathing + # TEST 0: File is not safe to delete, due not being a temp dir or under FAST_PATH filePath0 = "test0_cleanPathNoMpi" open(filePath0, "w").write("something") - self.assertTrue(os.path.exists(filePath0)) with self.assertRaises(Exception): pathTools.cleanPath(filePath0, mpiRank=0) - # TEST 1: Delete a single file - filePath1 = "test1_cleanPathNoMpi_mongoose" + # TEST 1: Delete a single file under FAST_PATH + filePath1 = os.path.join(context.getFastPath(), "test1_cleanPathNoMpi") open(filePath1, "w").write("something") - self.assertTrue(os.path.exists(filePath1)) pathTools.cleanPath(filePath1, mpiRank=0) self.assertFalse(os.path.exists(filePath1)) - # TEST 2: Delete an empty directory - dir2 = "mongoose" + # TEST 2: Delete an empty directory under FAST_PATH + dir2 = os.path.join(context.getFastPath(), "letitgo") os.mkdir(dir2) - self.assertTrue(os.path.exists(dir2)) pathTools.cleanPath(dir2, mpiRank=0) self.assertFalse(os.path.exists(dir2)) - # TEST 3: Delete a directory with two files inside - # create directory - dir3 = "mongoose" + # TEST 3: Delete an empty directory with tempDir=True + dir3 = "noyoureadirectory" os.mkdir(dir3) - - # throw in a couple of simple text files - open(os.path.join(dir3, "file1.txt"), "w").write("something1") - open(os.path.join(dir3, "file2.txt"), "w").write("something2") - - # delete the directory and test self.assertTrue(os.path.exists(dir3)) - self.assertTrue(os.path.exists(os.path.join(dir3, "file1.txt"))) - self.assertTrue(os.path.exists(os.path.join(dir3, "file2.txt"))) - pathTools.cleanPath(dir3, mpiRank=0) + pathTools.cleanPath(dir3, mpiRank=0, tempDir=True) self.assertFalse(os.path.exists(dir3)) + # TEST 4: Delete a directory with two files inside with tempDir=True + dir4 = "dirplease" + os.mkdir(dir4) + open(os.path.join(dir4, "file1.txt"), "w").write("something1") + open(os.path.join(dir4, "file2.txt"), "w").write("something2") + # delete the directory and test + self.assertTrue(os.path.exists(dir4)) + self.assertTrue(os.path.exists(os.path.join(dir4, "file1.txt"))) + self.assertTrue(os.path.exists(os.path.join(dir4, "file2.txt"))) + pathTools.cleanPath(dir4, mpiRank=0, tempDir=True) + self.assertFalse(os.path.exists(dir4)) + def test_isFilePathNewer(self): with TemporaryDirectoryChanger(): path1 = "test_isFilePathNewer1.txt" diff --git a/pyproject.toml b/pyproject.toml index c1ec042115..9257098392 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -40,7 +40,7 @@ dependencies = [ "pluggy>=1.2.0", # Central tool behind the ARMI Plugin system "pyDOE>=0.3.8", # We import a Latin-hypercube algorithm to explore a phase space "pyevtk>=1.2.0", # Handles binary VTK visualization files - "ruamel.yaml ; python_version >= '3.11.0'", # Our foundational YAML library + "ruamel.yaml<0.19.0; python_version >= '3.11.0'", # Our foundational YAML library "ruamel.yaml.clib ; python_version >= '3.11.0'", # C-based core of ruamel below "ruamel.yaml.clib<=0.2.7 ; python_version < '3.11.0'", # C-based core of ruamel below "ruamel.yaml<=0.17.21 ; python_version < '3.11.0'", # Our foundational YAML library