FileCache: Delete the new cache file on exception (#34623)

The code in FileCache for write_transaction attempts to delete the temporary file when an exception occurs under the context by calling shutil.rmtree. However, rmtree only operates on directories while the rest of FileCache uses normal files. This causes an empty file to be written to the cache key when unneeded.

Use os.remove instead which operates on normal files.

Co-authored-by: Harmen Stoppels <harmenstoppels@gmail.com>
This commit is contained in:
Jonathon Anderson 2023-01-10 06:36:12 -06:00 committed by GitHub
parent f1b8bc97f0
commit 6879c35d1c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 22 additions and 2 deletions

View File

@ -32,6 +32,27 @@ def test_write_and_read_cache_file(file_cache):
assert text == "foobar\n"
@pytest.mark.skipif(sys.platform == "win32", reason="Locks not supported on Windows")
def test_failed_write_and_read_cache_file(file_cache):
"""Test failing to write then attempting to read a cached file."""
with pytest.raises(RuntimeError, match=r"^foobar$"):
with file_cache.write_transaction("test.yaml") as (old, new):
assert old is None
assert new is not None
raise RuntimeError("foobar")
# Cache dir should have exactly one (lock) file
assert os.listdir(file_cache.root) == [".test.yaml.lock"]
# File does not exist
assert not file_cache.init_entry("test.yaml")
# Attempting to read will cause a FileNotFoundError
with pytest.raises(FileNotFoundError, match=r"test\.yaml"):
with file_cache.read_transaction("test.yaml"):
pass
def test_write_and_remove_cache_file(file_cache):
"""Test two write transactions on a cached file. Then try to remove an
entry from it.

View File

@ -144,8 +144,7 @@ def __exit__(cm, type, value, traceback):
cm.tmp_file.close()
if value:
# remove tmp on exception & raise it
shutil.rmtree(cm.tmp_filename, True)
os.remove(cm.tmp_filename)
else:
rename(cm.tmp_filename, cm.orig_filename)