-
-
Notifications
You must be signed in to change notification settings - Fork 176
Tests leave load of junk in my temp-dir #298
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
Comments
That's part of the problem. Another part of the problem is that Would it also make sense for FSes to be automatically |
Agree that the temp directories should be cleaned up. I don't recall ever looking at them to debug, so that's no great loss. I've found that code in That said, it is a useful pattern. And clearly I do use I think Python3.4 made some major improvements regarding destructors that made them more reliable, and thats our minimum Python3 version. I don't intend to support Python2.7 for much longer. So perhaps adding an auto-closing |
Fixed in v2.4.9 |
So I know this is a bit late but I just saw this issue and I think doing the fix in Instead, I suggest we should use |
I don't think we can avoid the I can see the use case for abandoning writing an archive if there is an error. I think it is is peculiar to archives. How about adding a parameter to with open_fs("zip://foo.zip", create=True) as zip_fs:
try:
write_files_to_zip(zip_fs)
except:
zip_fs.close(save=False) This would actually call @althonos may be interested in this conversation. |
Your example would put the onus on the user to enforce the safe behavior, but I think you're onto something. I'm tinkering with an implementation of some file systems (FAT, ext2, etc.) so people can read/write disk images. They essentially behave very similar to archives, so I have a vested interest in this. 🙂 |
Maybe this is another attribute that https://pyfilesystem2.readthedocs.io/en/latest/reference/base.html#fs.base.FS.getmeta could be used for? |
@willmcgugan I'm not sure what I'm suggesting, just throwing out random ideas ;-) Maybe an 'only _updates_on_close' or something? 🤷♂️
Hmm, in that case maybe it's worth making it an init-param, rather than a close-param (IYSWIM) ? Seems likely to me that you'd want all places where 'close' is called on an FS to have consistent behaviour? EDIT: ignore my "maybe it's worth making it an init-param, rather than a close-param" comment - I made it while tired and I now realise that the |
IMHO this issue should be re-opened. I've just re-run the commands from my initial comment in this thread on the latest @dargueta the "don't close an FS after an exception" discussion should probably be moved to a separate issue? |
Will do |
Pushed some changes to master which should delete the remaining tmp files. |
Observe:
Seems like
make_fs
in https://github.com/PyFilesystem/pyfilesystem2/blob/master/tests/test_osfs.py doestempfile.mkdtemp("fstestosfs")
but this directory never gets deleted anywhere :( Was there supposed to be anothershutil.rmtree
indestroy_fs
?I guess in some situations you may want to keep these temporary test directories for later debugging purposes, but IMHO running the unit-tests should auto-delete all these junk files by default, and only leave them behind if some kind of env-var or something is explicitly set?
The text was updated successfully, but these errors were encountered: