Added Files using tenacity for retrying renaming a directory on a opened filepath. #1826
Conversation
| "tomli >= 2.0, < 3.0; python_version <= '3.10'", | ||
| "tomli_w >= 1.0, < 2.0", | ||
| "tomli_w >= 1.0, < 2.0" |
There was a problem hiding this comment.
Comma on the end is intentional.
| "tomli_w >= 1.0, < 2.0" | |
| "tomli_w >= 1.0, < 2.0", |
| def test_rename_path(mock_tools, tmp_path): | ||
| def openclose(filepath): | ||
| handler = filepath.open(encoding="UTF-8") | ||
| time.sleep(1) |
There was a problem hiding this comment.
Sleep needs to be shorter - 1 second is a long time in a test
| def openclose(filepath): | ||
| handler = filepath.open(encoding="UTF-8") | ||
| time.sleep(1) | ||
| handler.close() |
There was a problem hiding this comment.
The close should be in a try:finally to ensure that if other parts of the process fail, the file will still be closed.
| rename_thread = threading.Thread( | ||
| target=mock_tools.files.path_rename, | ||
| args=(tmp_path / "orig-dir-1", tmp_path / "new-dir-1"), | ||
| ) |
There was a problem hiding this comment.
Is the rename thread needed? It should be possible to do this inline.
| file_access_thread.start() | ||
| rename_thread.start() | ||
|
|
||
| rename_thread.join() |
There was a problem hiding this comment.
We should join on the file_access_thread to ensure that it cleans up.
|
|
||
|
|
||
| @pytest.mark.xfail( | ||
| sys.platform == "win32", |
There was a problem hiding this comment.
I'm a little concerned about this being a win32 only error. macOS and Linux won't fail because of a file rename, but they could fail for other reasons - we need to verify the "will eventually fail" path on all platforms.
|
|
||
| Windows does not like renaming a dir in a path with an opened file. | ||
| """ | ||
| old_path.rename(new_path) |
There was a problem hiding this comment.
If this is a "retry on any error" handler - what happens for predictable failure - e.g., attempting to rename a file that doesn't exist? There are some failures that should be surfaced immediately.
|
Russell / Russell and Malcom, I will address all the above on this PR this weekend or early next week. Hope it's not too late. Work got backlogged for the past 7 days and unfortunately I'll need to catch up with it first... |
|
Just FYI: I ended up implementing the |
PermissionError: [WinError 5] Access denied#1780PR Checklist: