-
Notifications
You must be signed in to change notification settings - Fork 229
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix PermissionError when removing TemporaryDirectory on Windows #13
Comments
Can confirm that i am seeing a similar error |
I tried using |
See #11 and especially #8 for why I switched away from WeasyPrint. I don't have access to Windows, but this "Error" is actually a harmless warning (i.e. the resume.pdf file is successfully built) on Windows in Github Actions: https://github.com/mikepqr/resume.md/runs/2901233186?check_suite_focus=true. I believe it can be eliminated by upgrading to a more recent version of Python (e.g. 3.8, 3.9 or 3.10). Given it is harmless then I'd be glad to take a PR that hides it on 3.7, but I don't want to revert to WeasyPrint just to get rid of it. But like I said, I don't have access to Windows to test anything, so please let me know if any of the above is wrong (e.g. it's not harmless, it's not fixed by upgrading Python to 3.8+). |
According to https://bugs.python.org/issue26660, the snippet at https://docs.python.org/3/library/shutil.html?highlight=shutil#rmtree-example might help. I'd probably need to add that here Line 162 in f1b0699
|
Thanks for all the details @mikepqr . I'll try to make this change. |
Also, there are problems with WeasyPrint. :) |
Do note that, for me, it does not write the resume.pdf file.
|
OK. So i added the
|
No idea, I'm afraid. The error should be harmless to our goals (I think it's Chrome's own telemetry/logging tempfile/cleanup failing) but it raises which causes resume.py to receive the exception. You could try catching Line 149 in f1b0699
|
OK. So i re-approached the problem from a different angle, and it works 😀
I will raise a PR tomorrow as its quite late here 😄 |
OK, sounds promising! Also sounds like it might break macOS and Linux, so I will be sure to run rests on the PR 😄 |
I think it will not break for macOS and Linux because there are two separate functions now - one for win32 and another for macOS/Linux. But yes, you must test it, especially because I have not. |
We were getting errors like `PermissionError: [WinError 5] Access is denied: 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\resume.md_k098zhaj\\CrashpadMetrics-active.pma'` and `Exception ignored in: <finalize object at 0x16d9e039d60; dead> ... NotADirectoryError: [WinError 267] The directory name is invalid: 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\resume.md_3aoun3f2\\CrashpadMetrics-active.pma'` in CI on Windows. See e.g. https://github.com/mikepqr/resume.md/runs/5172290981 and https://github.com/mikepqr/resume.md/runs/5172234014. The root cause was that Chrome creates a file the python process does not have permission to delete. See puppeteer/puppeteer#2778. Because TemporaryDirectory is intended to be used as a context manager there is no way to prevent it logging an error when cleanup fails. The fix is to switch to the lower level tempfile.mkdtemp, and make a good faith attempt to clean it up manually, logging failure at the debug level (while adding a new --debug option). A more sophisticated fix would be to backport the new ignore_cleanup_errors option added in python 3.10 (python/cpython#24793), but this will do. Fixes #13
We were getting errors like `PermissionError: [WinError 5] Access is denied: 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\resume.md_k098zhaj\\CrashpadMetrics-active.pma'` and `Exception ignored in: <finalize object at 0x16d9e039d60; dead> ... NotADirectoryError: [WinError 267] The directory name is invalid: 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\resume.md_3aoun3f2\\CrashpadMetrics-active.pma'` in CI on Windows. See e.g. https://github.com/mikepqr/resume.md/runs/5172290981 and https://github.com/mikepqr/resume.md/runs/5172234014. The root cause was that Chrome creates a file the python process does not have permission to delete. See puppeteer/puppeteer#2778. Because TemporaryDirectory is intended to be used as a context manager there is no way to prevent it logging an error when cleanup fails. The fix is to switch to the lower level tempfile.mkdtemp, and make a good faith attempt to clean it up manually, logging failure at the debug level (while adding a new --debug option). A more sophisticated fix would be to backport the new ignore_cleanup_errors option added in python 3.10 (python/cpython#24793), but this will do. Fixes #13
For anyone else having this issue, you can find a recipe for a work-around in: |
I don't know if this only happens when it runs as a Github Action, or if it happens on regular Windows, but this is a pretty scary message for Windows users to see.
The access denied error is due to https://bugs.python.org/issue26660 (fixed in 3.8, but the Github windows environment uses 3.7, and I would rather not require 3.8 for users).
I am not sure why the exception we catch is bubbling up as "Exception ignored".
The text was updated successfully, but these errors were encountered: