From 3da01a3a67dc5b22a18d924cf002ed0f54aaf77e Mon Sep 17 00:00:00 2001 From: "Samantha C. Niemoeller" Date: Mon, 9 Mar 2026 17:43:47 -0700 Subject: [PATCH 1/2] Bugfix: Handle NFS silly rename issue when cleaning up the scratch directory --- src/nisarqa/utils/utils.py | 41 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/src/nisarqa/utils/utils.py b/src/nisarqa/utils/utils.py index 65b9331..a31f15e 100644 --- a/src/nisarqa/utils/utils.py +++ b/src/nisarqa/utils/utils.py @@ -1,5 +1,6 @@ from __future__ import annotations +import errno import logging import os import shutil @@ -755,10 +756,46 @@ def create_unique_subdirectory( try: yield path finally: - log = nisarqa.get_logger() if delete: + log = nisarqa.get_logger() + # Use onerror callback to handle NFS "silly rename" behavior + # See: https://github.com/isce-framework/nisarqa/issues/152 + + # shutil.rmtree's onerror was deprecated in Python 3.12 and was + # planned to be removed in 3.14, but its deprecation was changed + # to docs only, with no pending removal version. + # See: https://github.com/python/cpython/pull/118947 + def onerror(func, error_path, exc_info): + """Error handler for shutil.rmtree to handle NFS issues.""" + exc_type, exc_value, exc_tb = exc_info + + if issubclass(exc_type, FileNotFoundError): + # File was already deleted (possibly by NFS), ignore + pass + elif issubclass(exc_type, OSError): + # Check if this is a "Directory not empty" error + # This can occur on NFS due to "silly rename" (.nfsXXXXX files) + if exc_value.errno in (errno.ENOTEMPTY, errno.EEXIST): + # Log but don't raise - allow clean exit on NFS + log.warning( + f"nisarqa Could not delete '{error_path}': " + f" {exc_value}. This may occur on Network File" + " Systems where files are temporarily renamed" + " before deletion." + ) + else: + # Some other OSError - raise it + log.error(f"Error deleting '{error_path}': {exc_value}") + raise + else: + # Unexpected error type - raise it + log.error( + f"Unexpected error deleting '{error_path}': {exc_value}" + ) + raise + try: - shutil.rmtree(path) + shutil.rmtree(path, onerror=onerror) except FileNotFoundError: msg = ( f"Created directory was '{path}', but it was" From 26595d9aae14882cbac6c7ea51b850098b6624a1 Mon Sep 17 00:00:00 2001 From: Sam Niemoeller Date: Wed, 11 Mar 2026 19:13:56 +0000 Subject: [PATCH 2/2] cleanup duplicate handling of FileNotFoundError --- src/nisarqa/utils/utils.py | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/nisarqa/utils/utils.py b/src/nisarqa/utils/utils.py index a31f15e..6d2c0ef 100644 --- a/src/nisarqa/utils/utils.py +++ b/src/nisarqa/utils/utils.py @@ -770,11 +770,20 @@ def onerror(func, error_path, exc_info): exc_type, exc_value, exc_tb = exc_info if issubclass(exc_type, FileNotFoundError): - # File was already deleted (possibly by NFS), ignore - pass - elif issubclass(exc_type, OSError): - # Check if this is a "Directory not empty" error - # This can occur on NFS due to "silly rename" (.nfsXXXXX files) + # Directory was already deleted. Notify users because likely + # they are violating a nisarqa design pattern; they + # should fix their code to prevent tricky runtime bugs. + msg = ( + f"Created directory was '{path}', but it was" + " deleted external to (but within the context of)" + " this context manager." + ) + log.error(msg) + raise FileNotFoundError(msg) + + if issubclass(exc_type, OSError): + # Check if this is a "Directory not empty" error. This can + # occur on NFS due to "silly rename" (.nfsXXXXX files) if exc_value.errno in (errno.ENOTEMPTY, errno.EEXIST): # Log but don't raise - allow clean exit on NFS log.warning( @@ -794,18 +803,9 @@ def onerror(func, error_path, exc_info): ) raise - try: - shutil.rmtree(path, onerror=onerror) - except FileNotFoundError: - msg = ( - f"Created directory was '{path}', but it was" - " deleted external to (but within the context of)" - " this context manager." - ) - log.error(msg) - raise FileNotFoundError(msg) - else: - log.info(f"Directory deleted recursively: '{path}'") + # Delete directory. Exceptions will be handled via onerror. + shutil.rmtree(path, onerror=onerror) + log.info(f"Directory deleted recursively: '{path}'") def set_global_scratch_dir(scratch_dir: str | os.PathLike) -> None: