Added Files using tenacity for retrying renaming a directory on a opened filepath.#1826
Added Files using tenacity for retrying renaming a directory on a opened filepath.#1826spa51273 wants to merge 2 commits intobeeware:mainfrom
Conversation
freakboy3742
left a comment
freakboy3742
left a comment
There was a problem hiding this comment.
Thanks for the PR - the broad structure looks good; I've got some comments inline.
| "tenacity >= 8.0, < 9.0", | ||
| "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 |
|
@spa51273 are you still working on this? |
|
Jonathan,
No i am not. So sorry this has been off my radar for quite a while..
…On Sat, Mar 7, 2026, 6:12 PM Johanan Oppong Amoateng < ***@***.***> wrote:
*JohananOppongAmoateng* left a comment (beeware/briefcase#1826)
<#1826 (comment)>
@spa51273 <https://github.com/spa51273> are you still working on this?
—
Reply to this email directly, view it on GitHub
<#1826 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMR5MO6BDSK6NWK7OK5HII34PS3IVAVCNFSM6AAAAACWKUYPIOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DAMJXG4YTMNZYGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Superseded by #2725. |
5 participants
PermissionError: [WinError 5] Access denied#1780PR Checklist: